[ISSUE #3532]🚀Implement missing methods in DefaultHAConnection for state management and data transfer#3533
Conversation
…ate management and data transfer
|
🔊@mxsm 🚀Thanks for your contribution🎉! 💡CodeRabbit(AI) will review your code first🔥! Note 🚨The code review suggestions from CodeRabbit are to be used as a reference only, and the PR submitter can decide whether to make changes based on their own judgment. Ultimately, the project management personnel will conduct the final code review💥. |
There was a problem hiding this comment.
Pull Request Overview
This PR implements previously missing data transfer methods in the HAConnection trait for DefaultHAConnection and removes redundant inherent getters.
- Removed obsolete inherent getter methods for state and transfer metrics.
- Added implementations for
get_transferred_byte_in_second,get_transfer_from_where, andget_slave_ack_offsetin theHAConnectiontrait impl.
Comments suppressed due to low confidence (3)
rocketmq-store/src/ha/default_ha_connection.rs:208
- It looks like the trait
HAConnectionmay also require aget_current_statemethod, but it's not implemented here; please add this to avoid a compile error and restore state management.
}
rocketmq-store/src/ha/default_ha_connection.rs:210
- [nitpick] Consider adding unit tests for
get_transferred_byte_in_second,get_transfer_from_where, andget_slave_ack_offsetto verify correct behavior under various load and error conditions.
fn get_transferred_byte_in_second(&self) -> i64 {
rocketmq-store/src/ha/default_ha_connection.rs:210
- [nitpick] These trait methods lack documentation; adding doc comments would clarify their purpose and expected return values for future maintainers.
fn get_transferred_byte_in_second(&self) -> i64 {
WalkthroughThe code removes redundant inherent getter methods from the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant DefaultHAConnection
participant FlowMonitor
participant WriteSocketService
Client->>DefaultHAConnection: get_current_state()
DefaultHAConnection->>DefaultHAConnection: read state from RwLock
DefaultHAConnection-->>Client: HAConnectionState
Client->>DefaultHAConnection: get_slave_ack_offset()
DefaultHAConnection-->>Client: slave_ack_offset
Client->>DefaultHAConnection: get_transferred_byte_in_second()
DefaultHAConnection->>FlowMonitor: get_transferred_byte_in_second()
FlowMonitor-->>DefaultHAConnection: value
DefaultHAConnection-->>Client: value
Client->>DefaultHAConnection: get_transfer_from_where()
DefaultHAConnection->>WriteSocketService: get_next_transfer_offset()
WriteSocketService-->>DefaultHAConnection: offset or None
DefaultHAConnection-->>Client: offset or -1
Assessment against linked issues
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code Graph Analysis (1)rocketmq-store/src/ha/default_ha_connection.rs (3)
⏰ Context from checks skipped due to timeout of 90000ms (9)
🔇 Additional comments (4)
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3533 +/- ##
=======================================
Coverage 26.18% 26.18%
=======================================
Files 556 556
Lines 78651 78642 -9
=======================================
Hits 20593 20593
+ Misses 58058 58049 -9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Which Issue(s) This PR Fixes(Closes)
Fixes #3532
Brief Description
How Did You Test This Change?
Summary by CodeRabbit