[ISSUE #6618]🚀Add hash-based message queue selector#6619
[ISSUE #6618]🚀Add hash-based message queue selector#6619rocketmq-rust-bot merged 1 commit intomainfrom
Conversation
|
🔊@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💥. |
WalkthroughIntroduces a new hash-based message queue selector feature to the RocketMQ Rust client. Implements Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
rocketmq-client/benches/select_queue_benchmark.rs (1)
54-70: Consider using::new()for consistency.Line 55 uses
SelectMessageQueueByHashdirectly while the unit tests useSelectMessageQueueByHash::new(). Both work, but::new()is more explicit and consistent with the test code.♻️ Suggested change
fn bench_select_original(c: &mut Criterion) { - let selector = SelectMessageQueueByHash; + let selector = SelectMessageQueueByHash::new();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rocketmq-client/benches/select_queue_benchmark.rs` around lines 54 - 70, Replace the direct type value with an explicit constructor to match tests: in bench_select_original create the selector using SelectMessageQueueByHash::new() instead of referencing SelectMessageQueueByHash directly; update the variable initialization for selector where SelectMessageQueueByHash is used so it calls the new() method.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@rocketmq-client/benches/select_queue_benchmark.rs`:
- Around line 1-13: The source file is missing the required copyright header;
add the repository-standard header containing "Copyright 2023" at the very top
of the Rust file (above the first use statements such as use
criterion::criterion_group and use
rocketmq_client_rust::producer::message_queue_selector::MessageQueueSelector) so
every .rs file includes the mandated license header.
---
Nitpick comments:
In `@rocketmq-client/benches/select_queue_benchmark.rs`:
- Around line 54-70: Replace the direct type value with an explicit constructor
to match tests: in bench_select_original create the selector using
SelectMessageQueueByHash::new() instead of referencing SelectMessageQueueByHash
directly; update the variable initialization for selector where
SelectMessageQueueByHash is used so it calls the new() method.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
rocketmq-client/benches/select_queue_benchmark.rsrocketmq-client/src/producer.rsrocketmq-client/src/producer/queue_selector.rsrocketmq-client/src/producer/queue_selector/select_message_queue_by_hash.rs
| use criterion::criterion_group; | ||
| use criterion::criterion_main; | ||
| use criterion::Criterion; | ||
| use rocketmq_client_rust::producer::message_queue_selector::MessageQueueSelector; | ||
| use rocketmq_client_rust::producer::queue_selector::SelectMessageQueueByHash; | ||
| use rocketmq_common::common::message::message_queue::MessageQueue; | ||
| use rocketmq_common::common::message::message_single::Message; | ||
| use std::cell::RefCell; | ||
| use std::collections::hash_map::DefaultHasher; | ||
| use std::hash::Hash; | ||
| use std::hash::Hasher; | ||
| use std::hint::black_box; | ||
|
|
There was a problem hiding this comment.
Add the copyright header.
This file is missing the required copyright header. Per repository convention, all Rust source files must include the "Copyright 2023" header.
📝 Proposed fix
+// Copyright 2023 The RocketMQ Rust Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
use criterion::criterion_group;Based on learnings: "In Rust source files (*.rs) across the rocketmq-rust repository, enforce using 'Copyright 2023' as the year in the header."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| use criterion::criterion_group; | |
| use criterion::criterion_main; | |
| use criterion::Criterion; | |
| use rocketmq_client_rust::producer::message_queue_selector::MessageQueueSelector; | |
| use rocketmq_client_rust::producer::queue_selector::SelectMessageQueueByHash; | |
| use rocketmq_common::common::message::message_queue::MessageQueue; | |
| use rocketmq_common::common::message::message_single::Message; | |
| use std::cell::RefCell; | |
| use std::collections::hash_map::DefaultHasher; | |
| use std::hash::Hash; | |
| use std::hash::Hasher; | |
| use std::hint::black_box; | |
| // Copyright 2023 The RocketMQ Rust Authors | |
| // | |
| // Licensed under the Apache License, Version 2.0 (the "License"); | |
| // you may not use this file except in compliance with the License. | |
| // You may obtain a copy of the License at | |
| // | |
| // http://www.apache.org/licenses/LICENSE-2.0 | |
| // | |
| // Unless required by applicable law or agreed to in writing, software | |
| // distributed under the License is distributed on an "AS IS" BASIS, | |
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | |
| // See the License for the specific language governing permissions and | |
| // limitations under the License. | |
| use criterion::criterion_group; | |
| use criterion::criterion_main; | |
| use criterion::Criterion; | |
| use rocketmq_client_rust::producer::message_queue_selector::MessageQueueSelector; | |
| use rocketmq_client_rust::producer::queue_selector::SelectMessageQueueByHash; | |
| use rocketmq_common::common::message::message_queue::MessageQueue; | |
| use rocketmq_common::common::message::message_single::Message; | |
| use std::cell::RefCell; | |
| use std::collections::hash_map::DefaultHasher; | |
| use std::hash::Hash; | |
| use std::hash::Hasher; | |
| use std::hint::black_box; | |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rocketmq-client/benches/select_queue_benchmark.rs` around lines 1 - 13, The
source file is missing the required copyright header; add the
repository-standard header containing "Copyright 2023" at the very top of the
Rust file (above the first use statements such as use criterion::criterion_group
and use
rocketmq_client_rust::producer::message_queue_selector::MessageQueueSelector) so
every .rs file includes the mandated license header.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6619 +/- ##
==========================================
+ Coverage 41.36% 41.40% +0.03%
==========================================
Files 964 965 +1
Lines 135273 135354 +81
==========================================
+ Hits 55956 56038 +82
+ Misses 79317 79316 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
rocketmq-rust-bot
left a comment
There was a problem hiding this comment.
LGTM - All CI checks passed ✅
Which Issue(s) This PR Fixes(Closes)
Brief Description
How Did You Test This Change?
Summary by CodeRabbit
Release Notes
New Features
Tests