Skip to content

[ISSUE #9339]♻️Inject remoting command factories into NameServer owners - #9340

Merged
mxsm merged 1 commit into
mainfrom
mxsm/inject-namesrv-command-factory
Aug 13, 2026
Merged

[ISSUE #9339]♻️Inject remoting command factories into NameServer owners#9340
mxsm merged 1 commit into
mainfrom
mxsm/inject-namesrv-command-factory

Conversation

@mxsm

@mxsm mxsm commented Aug 13, 2026

Copy link
Copy Markdown
Owner

Which Issue(s) This PR Fixes(Closes)

Brief Description

  • adds owner-scoped typed-error response construction to RemotingCommandFactory
  • retains one immutable command factory in each NameServer runtime and injects it into request processors
  • routes cluster-test lookups and broker notifications through the owning factory
  • preserves transport error helpers as application-default compatibility wrappers
  • adds JSON/ROCKETMQ owner-isolation and outbound request regression coverage

Compatibility is preserved for request and response codes, remarks, bodies, headers, opaque values, flags, and typed error mappings. Builder::new remains backward compatible by capturing the application factory once, while embedded owners can select independent remoting versions and serialization types. All direct production static command constructions and global transport error-helper bypasses are removed from the migrated NameServer paths.

How Did You Test This Change?

  • cargo test -p rocketmq-protocol --lib (1483 passed)
  • cargo test -p rocketmq-transport --lib (193 passed)
  • cargo test -p rocketmq-namesrv --lib -- --skip config::tests::test_namesrv_config (255 passed)
  • cargo test -p rocketmq-namesrv --all-features --lib -- --skip config::tests::test_namesrv_config (257 passed)
  • cargo clippy -p rocketmq-namesrv --all-targets --all-features -- -D warnings
  • cargo clippy -p rocketmq-protocol -p rocketmq-transport --all-targets --all-features -- -D warnings
  • cargo clippy --workspace --no-deps --all-targets --all-features -- -D warnings
  • all 28 root workspace packages passed package-scoped cargo fmt -p <package> -- --check
  • fixed-nightly fuzz build passed
  • standalone example Clippy passed
  • RocketMQ MCP read-only boundary, default/all-feature tests, streamable HTTP Clippy, and rustdoc passed
  • protocol/transport rustdoc, error hygiene, wire golden, and Java header compatibility checks passed

Aggregate cargo fmt --all -- --check reaches Windows OS error 206; package-scoped checks passed for all 28 workspace packages. config::tests::test_namesrv_config fails unchanged on main because its allow_insecure_public_listener assertion disagrees with the current default; all other NameServer default and all-feature tests pass.

Summary by CodeRabbit

  • New Features

    • Added configurable response settings for each NameServer, including response version and serialization format.
    • Added support for customizing how NameServer requests and responses are created.
    • Added consistent error responses with mapped error codes and optional remarks.
  • Bug Fixes

    • Ensured custom response settings are preserved across routing, errors, unsupported requests, and readiness checks.
    • Prevented response configuration from leaking between independent NameServer instances.

@rocketmq-rust-bot

Copy link
Copy Markdown
Collaborator

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

@mxsm
mxsm merged commit cc1291c into main Aug 13, 2026
33 of 40 checks passed
@coderabbitai

coderabbitai Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 47e8d4b0-b558-4fad-b927-f95477dcf2ee

📥 Commits

Reviewing files that changed from the base of the PR and between b3a866d and 3710c4e.

📒 Files selected for processing (10)
  • rocketmq-namesrv/src/bootstrap.rs
  • rocketmq-namesrv/src/processor.rs
  • rocketmq-namesrv/src/processor/client_request_processor.rs
  • rocketmq-namesrv/src/processor/cluster_test_request_processor.rs
  • rocketmq-namesrv/src/processor/cluster_test_request_processor/route_lookup.rs
  • rocketmq-namesrv/src/processor/default_request_processor.rs
  • rocketmq-namesrv/src/processor/response_factory.rs
  • rocketmq-namesrv/src/route/route_info_manager.rs
  • rocketmq-protocol/src/protocol/remoting_command_defaults.rs
  • rocketmq-transport/src/error_response.rs

Walkthrough

NameServer builders now accept and retain a RemotingCommandFactory. Processors and route-management components use the owner-scoped factory for remoting requests and responses. Typed-error factory methods centralize error mapping. Tests verify custom versions, serialization types, and owner isolation.

Changes

NameServer command factory injection

Layer / File(s) Summary
Factory response contracts
rocketmq-protocol/src/protocol/remoting_command_defaults.rs, rocketmq-transport/src/error_response.rs, rocketmq-namesrv/src/processor/response_factory.rs
Typed errors and standardized NameServer responses now use RemotingCommandFactory, while transport helpers delegate to the centralized methods.
Builder and runtime propagation
rocketmq-namesrv/src/bootstrap.rs
Builder initializes and accepts a factory. Runtime accessors expose it to request processors and cluster-test route lookup.
Processor response construction
rocketmq-namesrv/src/processor.rs, rocketmq-namesrv/src/processor/*_request_processor.rs
Request processors construct success, error, unsupported-request, and rejection responses through the stored factory. Tests verify custom metadata and independent NameServer defaults.
Outbound route commands
rocketmq-namesrv/src/processor/cluster_test_request_processor/route_lookup.rs, rocketmq-namesrv/src/route/route_info_manager.rs
Cluster-test route requests and minimum-broker-ID notifications use configured factory defaults. Tests verify version and serialization preservation.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Builder
  participant NameServerRuntime
  participant RequestProcessor
  participant RemotingCommandFactory
  participant RouteTransport

  Builder->>NameServerRuntime: configure RemotingCommandFactory
  NameServerRuntime->>RequestProcessor: provide owner-scoped factory
  RequestProcessor->>RemotingCommandFactory: create request or response
  RemotingCommandFactory-->>RequestProcessor: return configured command
  RequestProcessor->>RouteTransport: send encoded remoting command
Loading

Possibly related issues

  • mxsm/rocketmq-rust issue 9332 — Both changes replace static command construction with per-owner factories in different service owners.
  • mxsm/rocketmq-rust issue 9326 — Both changes preserve independent command version and serialization defaults for owner instances.
  • mxsm/rocketmq-rust issue 9334 — Both changes inject factories into request processors and route response construction through them.

Possibly related PRs

  • mxsm/rocketmq-rust#9322 — Provides the RemotingCommandFactory behavior extended here through NameServer processors and route handling.
  • mxsm/rocketmq-rust#9331 — Shares the per-owner factory propagation pattern for remoting requests and responses.
  • mxsm/rocketmq-rust#9338 — Uses owner-scoped factories in remoting servers and request processors.

Suggested reviewers: rocketmq-rust-bot, spacexcn, teslarustor

Poem

A rabbit wired commands with care,
Custom versions bloomed in air.
JSON and RocketMQ now run,
Each NameServer keeps its own.
Routes and replies follow the track,
No borrowed defaults sneak back.

✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch mxsm/inject-namesrv-command-factory

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Refactor♻️] Inject remoting command factories into NameServer owners

3 participants