Skip to content

Refactor join handler#7853

Merged
cjen1-msft merged 14 commits intomicrosoft:mainfrom
cjen1-msft:idempotent-join
May 1, 2026
Merged

Refactor join handler#7853
cjen1-msft merged 14 commits intomicrosoft:mainfrom
cjen1-msft:idempotent-join

Conversation

@cjen1-msft
Copy link
Copy Markdown
Contributor

Refactor to pull up the various early-exit checks.

The primary effect is that this ensures that we have a single edge in the join process.
Specifically if a node is requesting to join, if it ever is successful, it will always be successful (see the check_node_exists).
Previously we had the seqno check above that, which would mean if you hit different nodes in the network you'd possibly get different responses.

The other nice effect is that it makes the function a simple series of do we early exit and then join.

Refactor to pull up the various early-exit checks

Co-authored-by: Copilot <copilot@github.com>
Copilot AI review requested due to automatic review settings April 29, 2026 16:26
@cjen1-msft cjen1-msft requested a review from a team as a code owner April 29, 2026 16:26
Copy link
Copy Markdown
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

Refactors the /node/join handler in the node RPC frontend to consolidate early-exit checks and ensure consistent “already joined” behavior (ie, once a join succeeds for a node identity, subsequent joins return the same result rather than failing due to ordering of validation checks).

Changes:

  • Moves the “node already exists” check earlier so it short-circuits other validation (including the startup seqno gate).
  • Simplifies the join flow into: validate target/service → return existing-node response if present → redirect if non-primary → validate startup seqno → add node with status based on service state.
  • Replaces the is_taking_part_in_acking() usage with a direct NodeStatus::TRUSTED check.

Comment thread src/node/rpc/node_frontend.h
Comment thread src/node/rpc/node_frontend.h Outdated
@cjen1-msft cjen1-msft added the run-long-test Run Long Test job label Apr 30, 2026
Comment thread src/node/rpc/node_frontend.h Outdated
Comment thread src/node/rpc/node_frontend.h Outdated
Comment thread src/node/rpc/node_frontend.h Outdated
Comment thread src/node/rpc/node_frontend.h Outdated
@cjen1-msft cjen1-msft enabled auto-merge (squash) May 1, 2026 13:46
@cjen1-msft cjen1-msft merged commit 21b16d0 into microsoft:main May 1, 2026
19 checks passed
@cjen1-msft cjen1-msft deleted the idempotent-join branch May 5, 2026 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run-long-test Run Long Test job

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants