[ISSUES #909] Support AdminBrokerProcessor get_consumer_connection_list #920
Conversation
WalkthroughThe changes introduced significantly enhance the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Broker
participant ConsumerRequestHandler
Client->>Broker: Request consumer connection list
Broker->>ConsumerRequestHandler: Handle request
ConsumerRequestHandler->>ConsumerRequestHandler: Retrieve connection info
ConsumerRequestHandler-->>Broker: Return connection list
Broker-->>Client: Send response with connection list
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add Documentation and Community
|
|
🚀Thanks for your contribution🎉. CodeRabbit(AI) will review your code first🔥 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
Files selected for processing (7)
- rocketmq-broker/src/processor/admin_broker_processor.rs (3 hunks)
- rocketmq-broker/src/processor/admin_broker_processor/consumer_request_handler.rs (1 hunks)
- rocketmq-remoting/src/protocol/body.rs (1 hunks)
- rocketmq-remoting/src/protocol/body/connection.rs (1 hunks)
- rocketmq-remoting/src/protocol/body/consumer_connection.rs (1 hunks)
- rocketmq-remoting/src/protocol/header.rs (1 hunks)
- rocketmq-remoting/src/protocol/header/get_consumer_connection_list_request_header.rs (1 hunks)
Additional comments not posted (10)
rocketmq-remoting/src/protocol/body.rs (1)
23-25: Modules added successfully.The addition of
consumer_connectionandconnectionmodules enhances the protocol's capability to manage consumer connections.rocketmq-remoting/src/protocol/header.rs (1)
40-40: Module added successfully.The addition of
get_consumer_connection_list_request_headermodule supports handling consumer connection list requests.rocketmq-remoting/src/protocol/header/get_consumer_connection_list_request_header.rs (1)
24-51: Struct and methods implemented correctly.The
GetConsumerConnectionListRequestHeaderstruct is well-defined with appropriate serialization, deserialization, and utility methods.rocketmq-remoting/src/protocol/body/connection.rs (1)
22-72: Struct and methods implemented correctly.The
Connectionstruct is well-defined with appropriate methods for managing connection attributes. The use ofserdeis appropriate.rocketmq-broker/src/processor/admin_broker_processor/consumer_request_handler.rs (1)
30-90: Handler and method implemented correctly.The
ConsumerRequestHandlerstruct and its methodget_consumer_connection_listare well-implemented, effectively retrieving consumer connection lists and handling offline cases.rocketmq-remoting/src/protocol/body/consumer_connection.rs (1)
33-40: Good use ofArc<RwLock<>>for thread safety.The use of
Arc<RwLock<>>for shared mutable state is appropriate and ensures thread safety.rocketmq-broker/src/processor/admin_broker_processor.rs (4)
34-42: Import ofConsumerRequestHandleris appropriate.The import of
ConsumerRequestHandleris necessary for handling consumer-related requests in theAdminBrokerProcessor.
49-49: Addition ofConsumerRequestHandlertoAdminBrokerProcessoris consistent.The inclusion of
ConsumerRequestHandleraligns with the goal of managing consumer connections.
80-84: Constructor initialization ofConsumerRequestHandleris correct.The
AdminBrokerProcessor::newmethod correctly initializes theConsumerRequestHandleralong with other handlers.
138-142: New request case forGetConsumerConnectionListis well-integrated.The addition of the
RequestCode::GetConsumerConnectionListcase allows theAdminBrokerProcessorto handle consumer connection requests effectively.
| impl ConsumerConnection { | ||
| pub fn get_connection_set(&self) -> HashSet<Connection> { | ||
| self.connection_set.clone() | ||
| } | ||
|
|
||
| pub fn set_connection_set(&mut self, connection_set: HashSet<Connection>) { | ||
| self.connection_set = connection_set; | ||
| } | ||
|
|
||
| pub fn get_subscription_table(&self) -> HashMap<String, SubscriptionData> { | ||
| self.subscription_table.read().clone() | ||
| } | ||
|
|
||
| pub fn set_subscription_table( | ||
| &mut self, | ||
| subscription_table: HashMap<String, SubscriptionData>, | ||
| ) { | ||
| *self.subscription_table.write() = subscription_table; | ||
| } | ||
|
|
||
| pub fn get_consume_type(&self) -> ConsumeType { | ||
| *self.consume_type.read() | ||
| } | ||
|
|
||
| pub fn set_consume_type(&mut self, consume_type: ConsumeType) { | ||
| *self.consume_type.write() = consume_type; | ||
| } | ||
|
|
||
| pub fn get_message_model(&self) -> MessageModel { | ||
| *self.message_model.read() | ||
| } | ||
|
|
||
| pub fn set_message_model(&mut self, message_model: MessageModel) { | ||
| *self.message_model.write() = message_model; | ||
| } | ||
|
|
||
| pub fn get_consume_from_where(&self) -> ConsumeFromWhere { | ||
| *self.consume_from_where.read() | ||
| } | ||
|
|
||
| pub fn set_consume_from_where(&mut self, consume_from_where: ConsumeFromWhere) { | ||
| *self.consume_from_where.write() = consume_from_where; | ||
| } | ||
| } |
There was a problem hiding this comment.
Consider returning references in getters to avoid cloning.
Returning references instead of cloning the data in getters can improve performance by avoiding unnecessary data duplication. Consider changing the return types to references where applicable.
For example, you can change get_connection_set to return &HashSet<Connection>:
pub fn get_connection_set(&self) -> &HashSet<Connection> {
&self.connection_set
}
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #920 +/- ##
==========================================
- Coverage 24.18% 24.02% -0.16%
==========================================
Files 350 355 +5
Lines 28260 28437 +177
==========================================
- Hits 6834 6833 -1
- Misses 21426 21604 +178 ☔ View full report in Codecov by Sentry. |
Which Issue(s) This PR Fixes(Closes)
Fixes #909
Brief Description
How Did You Test This Change?
Summary by CodeRabbit
New Features
Bug Fixes
Documentation