Repository navigation
migration: complete target promotion catalog state - #1090
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: 🟠 High · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 13.69% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 482 functions across 60 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check タイトルは、PRの主要目的であるターゲット昇格完了時のカタログ状態更新を明確に示しています。 Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Description Check Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
- - [ ] Create stacked PR - [ ] Commit on current branch
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 @coderabbitai help to get the list of available commands.
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 (351477d):
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 |
|
To use Codex here, create a Codex account and connect to github. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the target promotion completion logic for split job migrations, including the state transition function CompleteTargetPromotionState and the transactional catalog store method CompleteSplitJobTargetPromotion. Feedback suggests strengthening defensive checks by validating the route's parent ID and start key during route clearing, and removing a redundant history lookup that could impact performance.
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.
| if !route.StagedVisibilityActive || route.GroupID != job.TargetGroupID { | ||
| return nil, errors.WithStack(ErrMigrationInvalidRoute) | ||
| } |
There was a problem hiding this comment.
To enforce defensive programming and ensure catalog consistency, we should also validate that the target route's ParentRouteID and Start key match the split job's SourceRouteID and SplitKey before clearing the staged visibility fields. This prevents accidentally promoting an inconsistent or incorrect route descriptor.
| if !route.StagedVisibilityActive || route.GroupID != job.TargetGroupID { | |
| return nil, errors.WithStack(ErrMigrationInvalidRoute) | |
| } | |
| if !route.StagedVisibilityActive || route.GroupID != job.TargetGroupID || route.ParentRouteID != job.SourceRouteID || !bytes.Equal(route.Start, job.SplitKey) { | |
| return nil, errors.WithStack(ErrMigrationInvalidRoute) | |
| } |
| if err := s.expectLiveSplitJobAt(ctx, expected.JobID, expectedRaw, readTS); err != nil { | ||
| return 0, 0, nil, err | ||
| } | ||
| if _, found, err := s.historySplitJobAt(ctx, expected.JobID, readTS); err != nil { | ||
| return 0, 0, nil, err | ||
| } else if found { | ||
| return 0, 0, nil, errors.WithStack(ErrCatalogSplitJobConflict) | ||
| } |
There was a problem hiding this comment.
The check s.historySplitJobAt is redundant and inefficient here. Since s.expectLiveSplitJobAt has already succeeded, the split job is guaranteed to be in the live state. In a consistent MVCC catalog, a split job cannot exist in both the live state and the history state simultaneously. Scanning the entire history of split jobs (which can grow very large in production) is expensive and unnecessary. We can safely remove this check.
| if err := s.expectLiveSplitJobAt(ctx, expected.JobID, expectedRaw, readTS); err != nil { | |
| return 0, 0, nil, err | |
| } | |
| if _, found, err := s.historySplitJobAt(ctx, expected.JobID, readTS); err != nil { | |
| return 0, 0, nil, err | |
| } else if found { | |
| return 0, 0, nil, errors.WithStack(ErrCatalogSplitJobConflict) | |
| } | |
| if err := s.expectLiveSplitJobAt(ctx, expected.JobID, expectedRaw, readTS); err != nil { | |
| return 0, 0, nil, err | |
| } |
References
- Remove dead or unreachable code paths, such as asynchronous handlers for tasks that are always performed synchronously, to improve code clarity and maintainability.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.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: 351477d4e1
ℹ️ 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".
| out.Changed = true | ||
| out.ClearedRouteIDs = cleared | ||
| out.Job.TargetPromotionDone = true | ||
| out.Job.PromotionCompletedTS = completionTS |
There was a problem hiding this comment.
In the promotion-complete path, this stores the caller-supplied completionTS in promotion_completed_ts, but the design contract for that field is the HLC commit timestamp of the default-group CAS that clears the descriptor (see docs/design/2026_06_11_proposed_hotspot_split_milestone2_migration.md, promotion_completed_ts). In normal calls the target-local promotion timestamp/max promoted timestamp can differ from the catalog MVCC commit timestamp chosen later in applyPromotionCompleteMutations, so target cleared-descriptor ACKs can prove or wait on the wrong timestamp; compute the catalog commit timestamp before encoding the job and persist that value instead.
Useful? React with 👍 / 👎.
351477d to
c57e42a
Compare
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 (c57e42a):
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. |
c57e42a to
2edefb8
Compare
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 (2edefb8):
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. |
|
Current head 2edefb8 has green CI and no current-head inline blockers found in the latest check. Requesting current-head review.\n\n@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs 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". |
|
@codex review Latest head has green CI and no current-head top-level inline findings from the review-comments API. Requesting a fresh review for the current head. |
|
Codex Review: Didn't find any major issues. Keep it up! 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". |
…oup' into design/hotspot-split-m2-promote
…ain' into design/hotspot-split-m2-cross-group
…oup' into design/hotspot-split-m2-promote
… into design/hotspot-split-m2-promotion-complete
|
@codex review Merged latest design/hotspot-split-m2-promote into this branch and fixed the promotion-complete compatibility with the updated catalog save plan/v2 route descriptor write gate in 9d9e051. Validation:
|
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 (9d9e051):
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9d9e051a7f
ℹ️ 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".
| if currentVersion != expectedVersion { | ||
| return 0, 0, nil, errors.WithStack(ErrCatalogVersionMismatch) | ||
| } | ||
| if err := s.expectLiveSplitJobAt(ctx, expected.JobID, expectedRaw, readTS); err != nil { |
There was a problem hiding this comment.
If a completion attempt commits but the caller loses the response, retrying the same request still carries the original catalog version and pre-completion SplitJob. The first attempt has already bumped the catalog version and updated the live job, so these checks return ErrCatalogVersionMismatch/ErrCatalogSplitJobConflict before the cleared-descriptor idempotent path can run; the retry reports failure for an already-applied promotion unless the caller reloads and resubmits the new version/job.
Useful? React with 👍 / 👎.
| if err := s.store.ApplyMutations(ctx, mutations, readKeys, plan.readTS, commitTS); err != nil { | ||
| if errors.Is(err, store.ErrWriteConflict) { | ||
| return errors.WithStack(ErrCatalogSplitJobConflict) |
There was a problem hiding this comment.
When another catalog save commits after loadPromotionCompleteInputs but before this batch applies, ApplyMutations will report a write conflict on CatalogVersionKey, but this branch wraps every such conflict as ErrCatalogSplitJobConflict. That makes a plain catalog-version race look like the split job changed, unlike CatalogStore.Save which maps version-key conflicts to ErrCatalogVersionMismatch, so callers cannot distinguish a route catalog retry from a real job-state conflict.
Useful? React with 👍 / 👎.
…eadiness' into design/hotspot-split-m2-job-rpc # Conflicts: # adapter/distribution_server_test.go
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 (7328918):
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 Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
Signed-off-by: bootjp <contact@bootjp.me>
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 (9975654):
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 HEAD: 9975654 Follow-up to the last push: the Ubuntu native-quorum acceptance test completed moving-size rejection, all-four-member native snapshot/restart and exact retained bytes, but exhausted its shared 60 s context on the final small retained write. It had forced a full 64 MiB snapshot on every applied entry. The fixture now snapshots every 8 entries and explicitly crosses one interval after the acknowledged retained write; the same all-member snapshot-index and restored-byte assertions remain. The deadline is still 60 s, direct no-quorum rejection and production Raft settings are unchanged. This adjustment passed twice under race checking (45.7 s total), and strict full lint reports 0 issues. Full repository race/short testing passed on the preceding commit, including adapter (437 s); only this test file changed afterwards. A 2-CPU/8-GiB local Linux VM passed the three quorum tests without race (26.3 s); its race attempts were OOM-killed, not passing evidence. Fresh final-HEAD Linux CI and regular/security review are required. DCO still reports an unknown verdict after failing to retrieve the 333-commit list; no override or history rewrite was performed. |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9975654dd7
ℹ️ 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".
| if err != nil { | ||
| return errors.WithStack(err) | ||
| } | ||
| _, err = client.CleanupMigration(ctx, request) |
There was a problem hiding this comment.
If a data-group leader accepts namespace activation but stops responding, this RPC can block the split runner indefinitely. The production path RunSplitJobRunner → prepareSplitJobWorkingSource → ensureMigrationNamespaces passes the process-lifetime context, and the client wrappers add credentials but no deadline. The newly added MODE8 activation closure bypasses cleanupSplitMigration, unlike the cleanup calls fixed earlier. Consequently, the serial runner cannot re-resolve leadership, retry activation, or process an operator's abandonment until restart. Use the bounded cleanup helper or an independently canceled attempt context here.
Useful? React with 👍 / 👎.
| if !resp.GetReady() || resp.GetAppliedConfigurationIndex() != req.GetExpectedConfigurationIndex() || | ||
| resp.GetTransferSizeGuardVersion() != store.MigrationTransferSizeGuardVersion { | ||
| return nil, errors.Errorf("transfer guard member %s lacks applied configuration or size admission ACK", member.ID) |
There was a problem hiding this comment.
When a responsive learner has not yet applied a newly committed source guard, probeMigrationControl legitimately returns Ready: false, but this branch converts that pending acknowledgement into an ordinary error. splitJobPhaseErrorRetryable classifies it as non-transient, so five consecutive attempts durably mark the migration FAILED instead of waiting for catch-up. In FENCE this can leave the healthy quorum's source write fence installed until an operator retries the job, even after the learner catches up. Unlike the previously repaired leader-loss case, the fresh evidence here is a successful probe response with an incomplete control ACK. Return or wrap errSplitMigrationVoterBarrierIncomplete for this condition so normal replication lag does not consume the terminal-failure budget.
Useful? React with 👍 / 👎.
Signed-off-by: bootjp <contact@bootjp.me>
Signed-off-by: bootjp <contact@bootjp.me>
|
@codex review HEAD: 5449c01 Addressed r4364403055 and r4364403062. MODE8 activation now uses the existing bounded cleanup RPC helper (2 s); regression tests cover a hung response, parent cancellation and an earlier parent deadline. A successful Ready:false member probe now wraps the incomplete-barrier sentinel, leaving the serial runner retryable. Nil responses, mismatched ready evidence and RPC authentication errors remain refused. Real store/FSM runner regressions exceed the five-attempt budget in both PLANNED and write-fenced FENCE, then resume automatically after learner catch-up. All helper callers were audited before changing error classification. The Ubuntu acceptance failure was exhaustion of one 60 s context shared across membership, large writes, four snapshots and restart. These phases now each have a bounded 60 s request budget under a 3 min cluster lifetime; startup/reopen cannot inherit an expired request budget. Exact byte, native snapshot-index, typed guard and 250 ms direct no-quorum assertions are unchanged. This is a test-fixture change, not a production deadline/consensus setting change. Integrated namespace/transfer/real-quorum race regressions passed (51.3 s), strict full lint: 0 issues. Fresh latest-HEAD CI is required. DCO still cannot retrieve the full PR commit list; no check override or history rewrite was used. No merge/deployment performed. |
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 (5449c01):
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5449c01a02
ℹ️ 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".
| if response.GetNamespaceAdmissionVersion() < kv.MigrationNamespaceVersion || | ||
| (configuration != 0 && response.GetAppliedConfigurationIndex() != configuration) || (activated && !response.GetReady()) { | ||
| return nil, errors.Wrapf(kv.ErrMigrationNamespaceProtocol, "namespace member %d support/configuration/activation unproven", member.MemberID) |
There was a problem hiding this comment.
When a learner has applied the current configuration but has not yet applied the MODE8 activation entry, probeMigrationNamespace legitimately returns the supported namespace version and matching configuration with Ready: false. The post-activation probe converts this replication lag into kv.ErrMigrationNamespaceProtocol, which splitJobPhaseErrorRetryable treats as terminal. Because namespace admission runs inside runSplitJobAttempt, five such ticks durably mark the job FAILED instead of resuming automatically after catch-up. Return the incomplete-barrier sentinel for a supported, configuration-matching response that is merely awaiting activation, while retaining refusal of unsupported protocol evidence.
Useful? React with 👍 / 👎.
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
Signed-off-by: bootjp <contact@bootjp.me>
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 (ec37113):
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 HEAD: ec37113 Addressed r4364500495: after protocol-version and configuration validation, a pending MODE8 activation ACK now returns the incomplete-barrier sentinel. Unsupported versions, mismatched configurations and RPC failures remain refused. Audited all namespace helper callers, including the split runner, partition refresh and main membership gate; none can advance on this error. The existing three-member final-ACK test now distinguishes learner replication lag from unsupported/configuration evidence. A real Pebble/Internal/FSM controller regression drives six pending namespace attempts through RunSplitJobRunnerOnce, proves no FAILED transition or geometry effect, then resumes BACKFILL after ACK. Both regressions fail on the previous implementation and pass after the fix. Namespace/transfer/phase-classification race tests passed (4.9 s); strict full lint: 0 issues. Fresh final-HEAD CI/review remain required. DCO is still an external unknown-verdict/commit-list-retrieval blocker; no override or history rewrite was used. |
|
Codex Review: Didn't find any major issues. Bravo. 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 Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
Signed-off-by: bootjp <contact@bootjp.me>
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 (7081873):
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 HEAD: 7081873 The preceding ec37113 implementation passed regular and security review. This push changes only .github/workflows/go-test.yml. The prior Ubuntu run completed serial acceptance tests, then exhausted the package-wide 10 min limit while the two existing 10,000-iteration gRPC sequence tests had run for only 2 min 22 s. Bulk race tests now exclude exactly those two anchored names, and the immediately following mandatory step runs those same full-length tests under race with a fresh, unchanged package budget. Both commands share one selection expression, so no test or iteration is removed; FUSE enforcement remains in the bulk step. YAML parsing and selection partition validation passed; go test -list confirms exactly the intended two names. Local full bulk/race and isolated Linux full-length/race verification are running. This is not a claim those pending runs or final CI already passed. Latest-HEAD regular/security review and CI are required. DCO remains unable to retrieve the full commit list; no override, history rewrite, merge or deployment was performed. |
|
Codex Review: Didn't find any major issues. Breezy! 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 Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
Resolve conflicts with the range-migration promotion/cleanup work (#1090): keep both the replicated-retention compaction fence and the migration retention floor in kv/compactor.go; route reserved opcodes through main's applyMigrationOpcode and then the branch's applyBackupOpcode; keep main's onePhaseStoreMutations/recordAndApplyOnePhase split, which already builds the absence-carrying store mutations; reject replicated control keys alongside main's migration-only control keys; and include both the replicated control namespace and migrationConfigurationIndexKey in the peer-local control predicate. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M143Q9MNkB4QTCD4Zbd1M8 Signed-off-by: Claude <noreply@anthropic.com>
## Scope Implementation of `docs/design/2026_10_17_proposed_ttl_inline_cross_leader_legacy_rows.md`, consolidated in this existing PR. The reviewed proposal remains Proposed until its runtime implementation and acceptance evidence are complete. This PR is intentionally draft during implementation. Base: `main`, integrated through `e9f69f88802bad2cc868fd8f0b7594917ca133eb`, including merged #1289 timestamp controls and #1291 shutdown handoff. The existing TTL-inline pass observations and metrics from merged #1283 are inherited rather than duplicated. No unmerged implementation is copied from another PR. The latest update resolves integration conflicts only and adds no new TTL feature or design scope. ## Progress - Integrated the separately tested legacy-metadata physical-page reporting foundation. - Atomic absence is implemented through protobuf/coordinator/1PC/store and remains refused by default until durable capability activation is implemented. Old-version/tombstone and same-entry conflict tests failed before the fix and passed afterward. - Delta and TTL migration plans share a timeout-aware top-level rotation. Completed classification snapshots, status and audit integration remain incomplete. - Implemented deterministic generation-tagged pin transitions, a durable state/index journal and a control-group identity allocator. Missing Release/Expire retire delayed admissions; stale expiry preserves an extended pin. Snapshot/reopen, failed-write, counter-exhaustion and encrypted-state regressions pass. Control records use the already-reserved transaction metadata namespace and preserve historical user keys. - Typed pin/allocation commands now have a strict decoder and actual FSM apply/restore paths. The highest admitted backup timestamp persists after release/expiry and fences delayed writes after restart. New-epoch state refuses legacy pin entries and cannot use legacy compaction while the real quorum/floor protocol is missing. There is no production producer or activation path for the new command format; a test-only seeded epoch/capability is not rollout evidence. - M1b capability/downgrade fencing, allocator fan-out, token/caller migration, durable drain/activation, read admission and compaction protection remain incomplete. The new protocol is not enabled by this incremental implementation. - M2 peer relocation RPC, owner-leader apply, contiguous-prefix checkpointing and seeded Jepsen validation remain incomplete. No migration milestone is declared complete by the foundation or by the earlier design review. Legacy TTL and dual-route point-read fallbacks remain enabled. ## Verification At `e8b21e81b7e577a3c1f0bef24e78ddccb6de4956`, integrated with main `e9f69f88`: full store/KV/distribution race suites pass on Go 1.27.1. The integration revision also passed adapter authentication/prepare-evidence forwarding/TTL migration/rotation tests and root prepare-evidence/shutdown tests three repetitions. The new memory/Pebble coexistence regression verifies both protocols through snapshot restore while retaining rejection of legacy backup lifecycle entries after pin-epoch activation. CI-version full-repository diff lint reports zero issues and diff check passes. The commit is signed, with bootjp <contact@bootjp.me> as author and committer. The integration review's P1 namespace finding is fixed using the existing store namespace predicate in export selection and the shared import/promote peer-local guard. Export, atomic import and restored-staging promotion regressions failed before the fix and pass after it. Repeated race coverage verifies raw journal/future control exclusion, preservation of the literal normalized user key and historical prepare-evidence user aliases, unchanged journal bytes across snapshot/reopen, no partial promotion/checkpoint advance, and reuse of a rejected import batch sequence for ordinary user data. No cursor logic or unmerged #1090 implementation is copied. The normal commit hook was attempted. The installed local linter cannot read Go 1.27 data. The CI linter's full-repository check reports 83 findings on both pristine main and the integrated tree; the integration-specific dispatch complexity was fixed, unrelated baseline files were not changed, and the hook was bypassed for this signed commit. CI-version diff lint and CI remain verification gates. Earlier-head evidence includes full adapter race (786.5 seconds) and about 74,000 wire-fuzz inputs; it is not presented as a current-head rerun. A [standalone manual review](#1280 (comment)) was requested after the latest push for this exact head. [Code review](#1280 (comment)) returned no major issues and [security review](#1280 (comment)) returned no issues. Current-head root inline findings: zero; the fixed P1 thread is replied to and resolved. Required build/lint/Ubuntu race checks, both Jepsen matrix legs and CodeQL pass. No CI failure or running check remains; the optional Claude checks are skipped. Merge state is CLEAN, but this incomplete PR remains draft and is not ready for merge. Main's prepare-evidence namespace and peer authentication are preserved, its existing migration guards are extended only for the already-reserved TTL control namespace, and the user's rolling-update.sh is untouched. No unmerged #1090 implementation was copied. Full protocol acceptance and seeded legacy-row migration Jepsen evidence remain incomplete; this update does not resume or expand M1b/M2 implementation. Author: bootjp
Design: docs/design/2026_10_23_proposed_migration_ownership_partitions.md
Author: bootjp
Current Review / CI
Published HEAD:
7081873ae20d8661ea734929dd83393d34f84d56.Fresh regular/security review and CI requested after the final push:
#1090 (comment)
Both reviews on this exact HEAD passed:
#1090 (comment)
#1090 (comment)
Executed test/build/static checks on this exact HEAD all passed, including both
Ubuntu race phases:
https://github.com/bootjp/elastickv/actions/runs/40030833765
DCO alone still fails with an unknown verdict; the all-checks-passing gate is
therefore not met. No merge has been performed.
The follow-up security finding r4363877733 is addressed: authenticate SplitRange
with the dedicated token over TLS and foreground-refresh disjoint serving native
owner closures before local route-engine publication/response. A real 1->2->3
regression reproduces the old disjoint write refusal and is now green without
background refresh; unsupported owner members defer the catalog mutation.
Independent watchers may briefly see committed C before data ACKs and retry;
this is not atomic cross-group catalog publication.
The normal review's r4364093051 is addressed: a persisted working child retries
the complete serving-owner barrier at committed C, without allocating routes or
advancing the catalog again. A fault-injected post-root-commit owner outage
reproduces the skipped ACK; recovery stays pending until that owner returns.
Move-back also replays target negative geometry without background refresh.
The next review's r4364403055 and r4364403062 are addressed: MODE8 uses the
existing two-second cleanup RPC helper; Ready:false transfer ACKs are retryable
incomplete barriers. PLANNED/FENCE runner regressions exceed five attempts,
preserve the write fence and resume automatically after learner catch-up.
Nil responses, mismatched ready evidence and RPC authentication errors remain
refused. The prior security review at
9975654dfound no issues; the final-HEADregular/security approvals are linked above.
The follow-up r4364500495 is also addressed: namespace activation ACK lag is an
incomplete barrier only after supported protocol and matching configuration are
proven. Three-member final-ACK and Pebble controller regressions distinguish it
from unsupported evidence, exceed the failure budget without geometry effects,
then resume BACKFILL after ACK. Namespace, partition-refresh and main membership
callers were audited; none advances on incomplete evidence.
Regular and security review both passed at
ec371135:#1090 (comment)
#1090 (comment)
The only subsequent change splits CI into bulk race tests and a mandatory
full-length gRPC sequence step. Both use one shared exact selection expression;
the two unchanged 9,999-iteration tests are moved, not skipped overall. The
prior package-wide 10 min budget expired after they had only run for 2 min 22 s.
Each phase now retains the original package budget, with no iteration reduction.
Final-HEAD executable CI has now passed; the DCO retrieval blocker remains.
The first Ubuntu failure used cached old leaders during large requests; current
leader re-resolution and an applied read barrier fix that harness defect, with
an explicit real leadership-transfer regression. The next run hit repeated
elections during race-instrumented 64 MiB applies on a small runner. The fixture
now models a 5 s election window and 100 ms heartbeats, rather than 1 s/10 ms.
The snapshot fixture now uses an eight-entry interval and explicitly crosses it
after the large write's ACK, instead of snapshotting 64 MiB after every entry.
Ubuntu still exhausted the single 60 s budget shared by all lifecycle phases.
Configuration/guard, large-write/snapshot and restart/admission now each have a
60 s request budget under a three-minute cluster lifetime. Node lifetimes do not
inherit canceled phase contexts. Explicit transfers, exact byte/size/all-member
snapshot checks and the direct 250 ms no-quorum write rejection are unchanged.
Production Raft, snapshot and RPC settings are unchanged. Final Linux CI passed;
this is not a production latency claim.
DCO is currently unable to evaluate this PR: the app's complete-commit-list
retrieval failed after REST returned 250 commits and its GraphQL fallback did
not complete. This is not a successful DCO verdict or a claim that all historic
signoffs passed. CLI re-run requests returned 404. The documented review-summary
recheck triggered a new DCO run at 22:08:37 UTC on 2026-10-23, which failed with
the same unknown verdict. No history rewrite, check override, merge or deployment
has been performed. App-side repair or explicitly authorized, backed-up history
consolidation is required before the all-checks-passing gate can be satisfied.
Changes
Validation
5449c01a(51.3 s); the subsequent namespace/transfer/phase-classification regression suite passed on the final Go source (4.9 s). Strict full lint reports 0 issues. Full short/race and recovery/move-back regressions also passed earlier.9975654d(26.0 s). Earlier VM race attempts were OOM-killed according to Docker; they are not passing race evidence or final-HEAD validation.572a209ftimed out after 12 min in two existing 10,000-iteration gRPC tests. An isolated rerun reproduced pending Raft proposals, including a Darwin WAL fsync; exact cause is unresolved. Those generic tests passed in Ubuntu CI at that HEAD. No skip or blind timeout increase was added.Five Self-Review Passes
Risks / Operation
Migration activation remains opt-in and requires coordinated binary support, peer authentication and opcode flags. Unsupported members or an unavailable native owner/target view defer the metadata mutation before geometry effects. Existing untransferable values abort the move before fencing; ordinary global-size behavior is unchanged. No production deployment is included.