Complete hotspot split M2 migration lifecycle - #1096
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
![gemini-code-assist[bot]](https://cdn.github.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a background split job runner to the DistributionServer that periodically identifies split jobs in the cleanup phase, promotes target-local staged rows, and completes the promotion via the coordinator. It also adds comprehensive unit tests and wires up the runner and its gRPC connection cache in main.go. Feedback on these changes suggests suppressing context.Canceled errors in the runner loop to avoid noisy logs during shutdown, and adding a check for empty addresses returned by RaftLeaderForKey in the client factory to fail fast with a descriptive error.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![chatgpt-codex-connector[bot]](https://cdn.github.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c065ad98fe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
@codex review Addressed current review findings:
|
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review Addressed the current-head split runner findings in 938a284:
Caller audit:
Validation:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 938a2842ec
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
TLA+ spec divergence review (auto-triggered)This PR touches files that the TLA+ safety spec has an anchor on (per Anchored files changed in this PR head (1c7ea5b):
What to check, by subsystem:
If the change is correct but requires a spec update, edit @claude review please verify TLA+ spec divergence per the checklist above. @codex review please verify TLA+ spec divergence per the checklist above. |
|
@codex review Addressed current-head promotion cleanup findings in 1c7ea5b:
Caller audit:
Validation:
|
|
To use Codex here, create a Codex account and connect to github. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c7ea5b343
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…n-complete' into design/hotspot-split-m2-runner
TLA+ spec divergence review (auto-triggered)This PR touches files that the TLA+ safety spec has an anchor on (per Anchored files changed in this PR head (7c1f0aa):
What to check, by subsystem:
If the change is correct but requires a spec update, edit @claude review please verify TLA+ spec divergence per the checklist above. @codex review please verify TLA+ spec divergence per the checklist above. |
|
@codex review Latest head 7c1f0aa addresses the promotion completion timestamp finding. Every transition to Done now records TerminalAtMs; already promoted jobs with a missing terminal time are normalized; both catalog and coordinator retry matchers ignore the completion-owned terminal timestamp when comparing the original expected job. Catalog, coordinator, and runner tests passed; full package compile passed; golangci-lint reported 0 issues; all production completion callers were audited. |
|
To use Codex here, create a Codex account and connect to github. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7c1f0aa431
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
TLA+ spec divergence review (auto-triggered)This PR touches files that the TLA+ safety spec has an anchor on (per Anchored files changed in this PR head (0c74b60):
What to check, by subsystem:
If the change is correct but requires a spec update, edit @claude review please verify TLA+ spec divergence per the checklist above. @codex review please verify TLA+ spec divergence per the checklist above. |
|
To use Codex here, create a Codex account and connect to github. |
|
@codex review Current head 0c74b60 implements the cross-group runner phases, durable source write tracking/fences, route-faithful lock drain, resumable bracket cursor imports, cutover guards, target promotion, and migration retention handling. Validation:
Please review the latest head; earlier review commits are stale. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0c74b6043d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
TLA+ spec divergence review (auto-triggered)This PR touches files that the TLA+ safety spec has an anchor on (per Anchored files changed in this PR head (634a75e):
What to check, by subsystem:
If the change is correct but requires a spec update, edit @claude review please verify TLA+ spec divergence per the checklist above. @codex review please verify TLA+ spec divergence per the checklist above. |
|
To use Codex here, create a Codex account and connect to github. |
bootjp
changed the title
TLA+ spec divergence review (auto-triggered)This PR touches files that the TLA+ safety spec has an anchor on (per Anchored files changed in this PR head (1cbee6a):
What to check, by subsystem:
If the change is correct but requires a spec update, edit @claude review please verify TLA+ spec divergence per the checklist above. @codex review please verify TLA+ spec divergence per the checklist above. |
|
To use Codex here, create a Codex account and connect to github. |
|
@codex review\n\nCurrent head 1cbee6a addresses the current-head P1 for 2PC commits prepared before tracker arm and committed after BACKFILL opens.\n\nCaller audit:\n- handleTxnRequest routes COMMIT only through handleCommitRequest.\n- raw, DEL_PREFIX, PREPARE, and one-phase paths already record their admitted write footprint.\n- COMMIT now records after commit mutation construction succeeds and before store apply.\n\nValidation:\n- go test ./kv -run 'TestFSMMigrationWriteTracker|TestFSMRejectsPrepareOnWriteFencedRouteButAllowsAbort|TestFSMAbortCleanup' -count=1\n- go test ./... -run '^$' -count=1\n- golangci-lint run ./... --timeout=5m --allow-parallel-runners\n- git diff --check\n- git verify-commit HEAD |
TLA+ spec divergence review (auto-triggered)This PR touches files that the TLA+ safety spec has an anchor on (per Anchored files changed in this PR head (8264ea3):
What to check, by subsystem:
If the change is correct but requires a spec update, edit @claude review please verify TLA+ spec divergence per the checklist above. @codex review please verify TLA+ spec divergence per the checklist above. |
|
To use Codex here, create a Codex account and connect to github. |
|
@codex review\n\nCurrent head 8264ea3 also addresses the two remaining current, non-outdated P2 threads:\n- migration capability and mutating migration RPCs now remain unavailable until the startup rotation/registration gate opens;\n- legacy DONE promotion retries persist TerminalAtMs normalization through the coordinator CAS.\n\nValidation:\n- go test . -run 'TestStartupPublicKVGate' -count=1\n- go test ./adapter -run 'TestDistributionServerCompleteSplitJobTargetPromotion' -count=1\n- go test ./distribution -run 'TestCompleteTargetPromotionState|TestCatalogStoreCompleteSplitJobTargetPromotion' -count=1\n- go test ./... -run '^$' -count=1\n- golangci-lint run ./... --timeout=5m --allow-parallel-runners\n- git verify-commit HEAD |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
28449f9
into
design/hotspot-split-m2-promotion-complete
## Summary - promote the hotspot split M2 migration design from partial to implemented - record the completed production runner, current-voter barriers, route cutover, cleanup, DONE history, and deterministic Jepsen workload - update the parent hotspot design and scaling roadmaps to the implemented path and current status ## Evidence - PR #1096 merged the final M2 runner/lifecycle slice into this stacked base - latest #1096 head review reported no major issues and all GitHub checks passed - targeted Go migration, voter barrier, cleanup, promotion, and route tests passed - `lein test elastickv.split-workload-test` passed (2 tests, 5 assertions) - old partial filename references are absent ## Remaining scope - M3 owns automatic hotspot detection, target selection, and split scheduling - M4 owns the broader route-shuffle nemesis matrix and production-scale fault campaigns - reverse migration after CUTOVER and concurrent migration jobs remain future extensions
1 participant
Summary
Safety
Validation
Go Test Results