Skip to content

[ISSUE #6328][Enhancement✨] Remove useless code from broker_outer_api.rs#6331

Merged
mxsm merged 1 commit intomxsm:mainfrom
oopscompiled:enhancement/remove-broker_api-comment
Feb 15, 2026
Merged

[ISSUE #6328][Enhancement✨] Remove useless code from broker_outer_api.rs#6331
mxsm merged 1 commit intomxsm:mainfrom
oopscompiled:enhancement/remove-broker_api-comment

Conversation

@oopscompiled
Copy link
Contributor

@oopscompiled oopscompiled commented Feb 14, 2026

Which Issue(s) This PR Fixes(Closes)

Brief Description

How Did You Test This Change?

Summary by CodeRabbit

  • Refactor
    • Optimized broker registration architecture by restructuring how registration tasks are delegated, improving internal system organization while maintaining existing functionality.

Copilot AI review requested due to automatic review settings February 14, 2026 23:53
@rocketmq-rust-bot
Copy link
Collaborator

🔊@oopscompiled 🚀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💥.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 14, 2026

Walkthrough

Removes a commented-out local broker registration call in favor of delegating to the broker runtime's outer API while maintaining concurrent task handling via Tokio join handles. No surrounding control flow changes.

Changes

Cohort / File(s) Summary
Broker Registration Refactoring
rocketmq-broker/src/out_api/broker_outer_api.rs
Removes a commented-out direct call to self.register_broker(...) and replaces it with delegation to broker_runtime_inner_.broker_outer_api().register_broker(...), shifting from local invocation to remoted invocation while preserving concurrent task spawning.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Suggested labels

approved, auto merge, Difficulty level/Easy, enhancement✨

Suggested reviewers

  • mxsm
  • SpaceXCN

Poem

🐰 A comment removed with a swish of the tail,
Useless code gone—hooray, all hail!
Delegation flows clean through the broker's core,
Tidier code means we need to store,
Less clutter, more flow—now that's what's for sure! ✨

🚥 Pre-merge checks | ✅ 5 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Merge Conflict Detection ⚠️ Warning ❌ Merge conflicts detected (175 files):

⚔️ .clippy.toml (content)
⚔️ .coderabbit.yaml (content)
⚔️ .github/workflows/dashboard-tauri-ci.yml (content)
⚔️ .github/workflows/deploy.yml (content)
⚔️ CHANGELOG.md (content)
⚔️ Cargo.lock (content)
⚔️ Cargo.toml (content)
⚔️ distribution/config/broker/1m-1s-async-one-machine/broker-master.toml (content)
⚔️ distribution/config/broker/1m-1s-async-one-machine/broker-slave.toml (content)
⚔️ distribution/config/broker/2m-2s-async/broker-a-master.toml (content)
⚔️ distribution/config/broker/2m-2s-async/broker-a-slave.toml (content)
⚔️ distribution/config/broker/2m-2s-async/broker-b-master.toml (content)
⚔️ distribution/config/broker/2m-2s-async/broker-b-slave.toml (content)
⚔️ distribution/config/broker/broker-simple-master.toml (content)
⚔️ distribution/config/broker/broker-simple-slave.toml (content)
⚔️ distribution/config/broker/broker.toml (content)
⚔️ distribution/config/nameserver/namesrv.toml (content)
⚔️ rocketmq-auth/Cargo.toml (content)
⚔️ rocketmq-auth/src/authentication/strategy.rs (content)
⚔️ rocketmq-auth/src/authorization/metadata_provider/local.rs (content)
⚔️ rocketmq-auth/src/authorization/model/acl.rs (content)
⚔️ rocketmq-auth/src/authorization/model/environment.rs (content)
⚔️ rocketmq-auth/src/authorization/model/policy.rs (content)
⚔️ rocketmq-auth/src/authorization/model/policy_entry.rs (content)
⚔️ rocketmq-auth/src/authorization/model/resource.rs (content)
⚔️ rocketmq-broker/Cargo.toml (content)
⚔️ rocketmq-broker/src/broker_runtime.rs (content)
⚔️ rocketmq-broker/src/client/consumer_group_info.rs (content)
⚔️ rocketmq-broker/src/long_polling/many_pull_request.rs (content)
⚔️ rocketmq-broker/src/out_api/broker_outer_api.rs (content)
⚔️ rocketmq-broker/src/processor.rs (content)
⚔️ rocketmq-broker/src/processor/consumer_manage_processor.rs (content)
⚔️ rocketmq-broker/src/processor/notification_processor.rs (content)
⚔️ rocketmq-broker/src/processor/pop_message_processor.rs (content)
⚔️ rocketmq-broker/src/processor/send_message_processor.rs (content)
⚔️ rocketmq-broker/src/topic/manager/topic_config_manager.rs (content)
⚔️ rocketmq-client/Cargo.toml (content)
⚔️ rocketmq-client/README.md (content)
⚔️ rocketmq-client/benches/thread_local_index_bench.rs (content)
⚔️ rocketmq-client/examples/broadcast/push_consumer.rs (content)
⚔️ rocketmq-client/examples/consumer/pop_consumer.rs (content)
⚔️ rocketmq-client/examples/ordermessage/ordermessage_consumer.rs (content)
⚔️ rocketmq-client/examples/quickstart/consumer.rs (content)
⚔️ rocketmq-client/src/admin/default_mq_admin_ext_impl.rs (content)
⚔️ rocketmq-client/src/base.rs (content)
⚔️ rocketmq-client/src/base/client_config.rs (content)
⚔️ rocketmq-client/src/common/thread_local_index.rs (content)
⚔️ rocketmq-client/src/consumer/consumer_impl/pull_api_wrapper.rs (content)
⚔️ rocketmq-client/src/consumer/default_mq_push_consumer.rs (content)
⚔️ rocketmq-client/src/consumer/default_mq_push_consumer_builder.rs (content)
⚔️ rocketmq-client/src/consumer/mq_push_consumer.rs (content)
⚔️ rocketmq-client/src/consumer/rebalance_strategy/allocate_message_queue_by_machine_room_nearby.rs (content)
⚔️ rocketmq-client/src/implementation/mq_client_api_impl.rs (content)
⚔️ rocketmq-client/src/producer.rs (content)
⚔️ rocketmq-client/src/producer/default_mq_produce_builder.rs (content)
⚔️ rocketmq-client/src/producer/default_mq_producer.rs (content)
⚔️ rocketmq-client/src/producer/mq_producer.rs (content)
⚔️ rocketmq-client/src/producer/producer_impl.rs (content)
⚔️ rocketmq-client/src/producer/producer_impl/default_mq_producer_impl.rs (content)
⚔️ rocketmq-client/src/producer/transaction_mq_producer.rs (content)
⚔️ rocketmq-client/tests/thread_local_index_test.rs (content)
⚔️ rocketmq-common/Cargo.toml (content)
⚔️ rocketmq-common/src/common.rs (content)
⚔️ rocketmq-common/src/common/broker/broker_config.rs (content)
⚔️ rocketmq-common/src/common/config_manager.rs (content)
⚔️ rocketmq-common/src/common/message.rs (content)
⚔️ rocketmq-common/src/common/topic.rs (content)
⚔️ rocketmq-common/src/lib.rs (content)
⚔️ rocketmq-common/src/utils/http_tiny_client.rs (content)
⚔️ rocketmq-controller/Cargo.toml (content)
⚔️ rocketmq-controller/examples/controller.toml (content)
⚔️ rocketmq-controller/src/cli.rs (content)
⚔️ rocketmq-controller/src/processor.rs (content)
⚔️ rocketmq-dashboard/rocketmq-dashboard-common/Cargo.toml (content)
⚔️ rocketmq-dashboard/rocketmq-dashboard-gpui/Cargo.lock (content)
⚔️ rocketmq-dashboard/rocketmq-dashboard-gpui/Cargo.toml (content)
⚔️ rocketmq-dashboard/rocketmq-dashboard-tauri/README.md (content)
⚔️ rocketmq-dashboard/rocketmq-dashboard-tauri/package-lock.json (content)
⚔️ rocketmq-dashboard/rocketmq-dashboard-tauri/package.json (content)
⚔️ rocketmq-dashboard/rocketmq-dashboard-tauri/src-tauri/Cargo.lock (content)
⚔️ rocketmq-dashboard/rocketmq-dashboard-tauri/src-tauri/Cargo.toml (content)
⚔️ rocketmq-dashboard/rocketmq-dashboard-tauri/src-tauri/src/lib.rs (content)
⚔️ rocketmq-dashboard/rocketmq-dashboard-tauri/src/pages/Login.tsx (content)
⚔️ rocketmq-doc/README.md (content)
⚔️ rocketmq-error/Cargo.toml (content)
⚔️ rocketmq-example/Cargo.lock (content)
⚔️ rocketmq-example/Cargo.toml (content)
⚔️ rocketmq-example/examples/consumer/pop_consumer.rs (content)
⚔️ rocketmq-filter/Cargo.toml (content)
⚔️ rocketmq-macros/Cargo.toml (content)
⚔️ rocketmq-namesrv/Cargo.toml (content)
⚔️ rocketmq-namesrv/resource/namesrv-example.toml (content)
⚔️ rocketmq-namesrv/resource/namesrv.toml (content)
⚔️ rocketmq-namesrv/src/route/route_info_manager.rs (content)
⚔️ rocketmq-namesrv/src/route/tables/topic_queue_mapping_table.rs (content)
⚔️ rocketmq-namesrv/src/route/types.rs (content)
⚔️ rocketmq-proxy/Cargo.toml (content)
⚔️ rocketmq-remoting/Cargo.toml (content)
⚔️ rocketmq-remoting/src/clients/rocketmq_tokio_client.rs (content)
⚔️ rocketmq-remoting/src/code.rs (content)
⚔️ rocketmq-remoting/src/code/request_code.rs (content)
⚔️ rocketmq-remoting/src/lib.rs (content)
⚔️ rocketmq-remoting/src/protocol.rs (content)
⚔️ rocketmq-remoting/src/protocol/admin/consume_stats.rs (content)
⚔️ rocketmq-remoting/src/protocol/admin/consume_stats_list.rs (content)
⚔️ rocketmq-remoting/src/protocol/admin/topic_stats_table.rs (content)
⚔️ rocketmq-remoting/src/protocol/body.rs (content)
⚔️ rocketmq-remoting/src/protocol/body/broker_body/register_broker_body.rs (content)
⚔️ rocketmq-remoting/src/protocol/body/broker_replicas_info.rs (content)
⚔️ rocketmq-remoting/src/protocol/body/check_client_request_body.rs (content)
⚔️ rocketmq-remoting/src/protocol/body/pop_process_queue_info.rs (content)
⚔️ rocketmq-remoting/src/protocol/body/process_queue_info.rs (content)
⚔️ rocketmq-remoting/src/protocol/body/queue_time_span.rs (content)
⚔️ rocketmq-remoting/src/protocol/body/timer_metrics_serialize_wrapper.rs (content)
⚔️ rocketmq-remoting/src/protocol/body/topic_info_wrapper/topic_config_wrapper.rs (content)
⚔️ rocketmq-remoting/src/protocol/body/topic_info_wrapper/topic_queue_wrapper.rs (content)
⚔️ rocketmq-remoting/src/protocol/body/unlock_batch_request_body.rs (content)
⚔️ rocketmq-remoting/src/protocol/header.rs (content)
⚔️ rocketmq-remoting/src/protocol/header/empty_header.rs (content)
⚔️ rocketmq-remoting/src/protocol/header/get_consumer_connection_list_request_header.rs (content)
⚔️ rocketmq-remoting/src/protocol/header/get_consumer_status_request_header.rs (content)
⚔️ rocketmq-remoting/src/protocol/header/get_max_offset_request_header.rs (content)
⚔️ rocketmq-remoting/src/protocol/header/get_max_offset_response_header.rs (content)
⚔️ rocketmq-remoting/src/protocol/header/get_min_offset_request_header.rs (content)
⚔️ rocketmq-remoting/src/protocol/header/get_topic_config_request_header.rs (content)
⚔️ rocketmq-remoting/src/protocol/header/get_topic_stats_request_header.rs (content)
⚔️ rocketmq-remoting/src/protocol/header/heartbeat_request_header.rs (content)
⚔️ rocketmq-remoting/src/protocol/header/namesrv/broker_request.rs (content)
⚔️ rocketmq-remoting/src/protocol/header/namesrv/perm_broker_header.rs (content)
⚔️ rocketmq-remoting/src/protocol/header/notification_response_header.rs (content)
⚔️ rocketmq-remoting/src/protocol/header/notify_broker_role_change_request_header.rs (content)
⚔️ rocketmq-remoting/src/protocol/header/notify_consumer_ids_changed_request_header.rs (content)
⚔️ rocketmq-remoting/src/protocol/header/query_consumer_offset_request_header.rs (content)
⚔️ rocketmq-remoting/src/protocol/header/query_consumer_offset_response_header.rs (content)
⚔️ rocketmq-remoting/src/protocol/header/query_topics_by_consumer_request_header.rs (content)
⚔️ rocketmq-remoting/src/protocol/heartbeat/subscription_data.rs (content)
⚔️ rocketmq-remoting/src/protocol/namespace_util.rs (content)
⚔️ rocketmq-remoting/src/protocol/namesrv.rs (content)
⚔️ rocketmq-remoting/src/protocol/route/topic_route_data.rs (content)
⚔️ rocketmq-remoting/src/protocol/static_topic.rs (content)
⚔️ rocketmq-remoting/src/protocol/static_topic/logic_queue_mapping_item.rs (content)
⚔️ rocketmq-remoting/src/protocol/static_topic/topic_config_and_queue_mapping.rs (content)
⚔️ rocketmq-remoting/src/protocol/static_topic/topic_queue_mapping_context.rs (content)
⚔️ rocketmq-remoting/src/protocol/static_topic/topic_queue_mapping_detail.rs (content)
⚔️ rocketmq-remoting/src/protocol/static_topic/topic_queue_mapping_one.rs (content)
⚔️ rocketmq-remoting/src/protocol/static_topic/topic_queue_mapping_utils.rs (content)
⚔️ rocketmq-remoting/src/protocol/static_topic/topic_remapping_detail_wrapper.rs (content)
⚔️ rocketmq-remoting/src/protocol/subscription/broker_stats_data.rs (content)
⚔️ rocketmq-remoting/src/protocol/subscription/group_retry_policy.rs (content)
⚔️ rocketmq-remoting/src/protocol/subscription/simple_subscription_data.rs (content)
⚔️ rocketmq-remoting/src/protocol/topic.rs (content)
⚔️ rocketmq-remoting/src/rpc/client_metadata.rs (content)
⚔️ rocketmq-runtime/Cargo.toml (content)
⚔️ rocketmq-store/Cargo.toml (content)
⚔️ rocketmq-store/src/utils/ffi.rs (content)
⚔️ rocketmq-tools/rocketmq-admin/rocketmq-admin-cli/Cargo.toml (content)
⚔️ rocketmq-tools/rocketmq-admin/rocketmq-admin-core/Cargo.toml (content)
⚔️ rocketmq-tools/rocketmq-admin/rocketmq-admin-core/src/admin/default_mq_admin_ext.rs (content)
⚔️ rocketmq-tools/rocketmq-admin/rocketmq-admin-core/src/admin/mq_admin_utils.rs (content)
⚔️ rocketmq-tools/rocketmq-admin/rocketmq-admin-core/src/cli/commands.rs (content)
⚔️ rocketmq-tools/rocketmq-admin/rocketmq-admin-core/src/commands.rs (content)
⚔️ rocketmq-tools/rocketmq-admin/rocketmq-admin-core/src/commands/auth_commands.rs (content)
⚔️ rocketmq-tools/rocketmq-admin/rocketmq-admin-core/src/commands/command_util.rs (content)
⚔️ rocketmq-tools/rocketmq-admin/rocketmq-admin-core/src/commands/consumer_commands.rs (content)
⚔️ rocketmq-tools/rocketmq-admin/rocketmq-admin-core/src/commands/topic_commands/topic_status_sub_command.rs (content)
⚔️ rocketmq-tools/rocketmq-admin/rocketmq-admin-tui/Cargo.toml (content)
⚔️ rocketmq-tools/rocketmq-store-inspect/Cargo.toml (content)
⚔️ rocketmq-website/PROJECT_STRUCTURE.md (content)
⚔️ rocketmq-website/PROJECT_STRUCTURE_zh-CN.md (content)
⚔️ rocketmq-website/README.md (content)
⚔️ rocketmq-website/package.json (content)
⚔️ rocketmq-website/releases/authors.yml (content)
⚔️ rocketmq/Cargo.toml (content)
⚔️ rust-toolchain.toml (content)
⚔️ rustfmt.toml (content)

These conflicts must be resolved before merging into main.
Resolve conflicts locally and push changes to this branch.
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: removing useless code from broker_outer_api.rs, which aligns with the actual modification of removing a commented-out code block.
Linked Issues check ✅ Passed The PR removes a commented-out code block as required by issue #6328's objective to remove useless code from broker_outer_api.rs.
Out of Scope Changes check ✅ Passed All changes are in-scope; the PR only modifies broker_outer_api.rs by removing commented code, directly addressing the linked issue without introducing unrelated modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
⚔️ Resolve merge conflicts (beta)
  • Auto-commit resolved conflicts to branch enhancement/remove-broker_api-comment
  • Post resolved changes as copyable diffs in a comment

No actionable comments were generated in the recent review. 🎉


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses issue #6328 by removing dead/commented-out code in BrokerOuterAPI::register_broker_all, keeping the broker registration flow clean and reducing noise in the implementation.

Changes:

  • Removed an obsolete commented-out let handle = ... block within the broker registration task spawn loop.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codecov
Copy link

codecov bot commented Feb 15, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 42.63%. Comparing base (58344a5) to head (0214013).
⚠️ Report is 105 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6331      +/-   ##
==========================================
+ Coverage   40.55%   42.63%   +2.08%     
==========================================
  Files         886      912      +26     
  Lines      122833   128056    +5223     
==========================================
+ Hits        49810    54592    +4782     
- Misses      73023    73464     +441     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Collaborator

@rocketmq-rust-bot rocketmq-rust-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - All CI checks passed ✅

Copy link
Owner

@mxsm mxsm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mxsm mxsm merged commit 63928a2 into mxsm:main Feb 15, 2026
17 of 27 checks passed
@rocketmq-rust-bot rocketmq-rust-bot added approved PR has approved and removed ready to review waiting-review waiting review this PR labels Feb 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Enhancement✨] Remove useless code from broker_outer_api.rs

5 participants