Skip metadata reconciliation after replica demotion

[mohanrajendran]

Summary

Recheck the node role inside the queued RunByMain callback before reconciling asynchronously fetched metadata. If the node was demoted to a replica while the fetch was in flight, discard the stale reconciliation and avoid issuing FT.INTERNAL_UPDATE from a read-only node.

The regression test deterministically queues the callback, demotes the node, and verifies that neither metadata callbacks nor FT.INTERNAL_UPDATE run.

Tests

• ./.build-debug/tests/coordinator_test --gtest_filter=*SkipsQueuedReconciliationAfterDemotion --gtest_brief=1
• ./.build-debug/tests/coordinator_test --gtest_brief=1 (44 tests passed)
• clang-format --dry-run --Werror src/coordinator/metadata_manager.cc testing/coordinator/metadata_manager_test.cc

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 7650fe19-cd10-49f8-98d6-7de886e13fe6

📥 Commits

Reviewing files that changed from the base of the PR and between 8fc3bbd and a2f70eb.

📒 Files selected for processing (2)

• src/coordinator/metadata_manager.cc
• testing/coordinator/metadata_manager_test.cc

Included review availability: Your plan includes up to 2 reviews per rolling hour; 1 remains after this review.

---

📝 Walkthrough

Walkthrough

Changes

Metadata demotion guard

Layer / File(s)	Summary
Demotion guard and regression coverage src/coordinator/metadata_manager.cc, testing/coordinator/metadata_manager_test.cc	The reconciliation callback checks whether the node is a replica before applying fetched metadata. The test verifies that queued reconciliation is discarded after demotion and that existing metadata remains unchanged.

Sequence Diagram(s)

sequenceDiagram
  participant AsyncMetadataRetrieval
  participant MetadataManager
  participant EventLoop
  participant ContextFlags
  AsyncMetadataRetrieval->>MetadataManager: deliver fetched metadata
  MetadataManager->>EventLoop: queue reconciliation callback
  ContextFlags->>MetadataManager: mark node as replica
  EventLoop->>MetadataManager: execute queued callback
  MetadataManager->>ContextFlags: check replica role
  MetadataManager-->>MetadataManager: discard reconciliation

Loading

Suggested reviewers: boda26

Merge Risk: ⚪ Minimal · up to a2f70

This change narrowly prevents stale metadata reconciliation after replica demotion, with targeted and full coordinator tests passing; no actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: skipping metadata reconciliation after replica demotion.
Description check	✅ Passed	The description directly explains the demotion handling change, regression test, and validation results.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[greptile-apps]

Greptile Summary

This change prevents a node that has been demoted to a replica from applying metadata fetched asynchronously while it was primary. The queued callback now rechecks the node role before reconciliation, and the added regression coverage models demotion between retrieval and callback dispatch.

Confidence Score: 5/5

Safe to merge: the demotion path retains existing metadata and suppresses reconciliation callbacks.

No blocking failure remains.

T-Rex Logs

What T-Rex did

• Ran the executable metadata demotion regression harness against the base and updated queued metadata callbacks to exercise the demotion flow.
• Compared the base run to the PR-head run; the updated callback places the guard before ReconcileMetadata, resulting in zero reconciliations and zero update callbacks and the existing metadata is retained.
• Used the trex-artifacts/demotion-gate-runtime-harness.py to verify guard placement and exercise the demotion dispatch model against base and PR-head configurations.
• Captured native-test blockers for the coordinator flow, including missing CMake prerequisites and the absence of a compiled coordinator test binary.

_{Ran code and verified through T-Rex}

_{Reviews (3): Last reviewed commit: "Merge branch 'main' into defensive-repli..." | Re-trigger Greptile}

[mohanrajendran]

/rerun

[Frank-Gu-81]

/label 1.3.0

[Aksha1812]

/assign-reviewers

[github-actions]

Reviewers for this PR

• First Pass Reviewer: @VoletiRam — Please do your best to do a detailed review on the PR and get a response on your feedback. Once the first pass is done, notify the maintainer assigned to this PR to follow up on the final review and getting the PR merged. You can reach out to the people owning the relevant code paths for more help on the review.
• Maintainer Reviewer: @allenss-amazon — Once the first review is done, please follow up with a final review and help to merge the change in.

_{Assigned automatically to the least-assigned members of the reviewer pools in .github/reviewer-pools.json. Use /reviewer or /remove-reviewer to adjust.}

[Aksha1812]

/reviewer BCathcart

[Aksha1812]

/reviewer Nivesh-01