Repository navigation
main: transfer Raft leadership before shutdown - #1291
Conversation
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 18 minutes. View limit details
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 |
|
Dependency/overlap check against the existing graceful-shutdown PR #1273:
The intended combined behavior should preserve #1273's early-startup/repeated-signal/clean-exit and FUSE guarantees while running this PR's voter handoffs before final root cancellation. Both standalone test suites passing would not establish that integration. Recording this here avoids a duplicate shutdown implementation or another PR for the same functions. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: f133f118a4
ℹ️ 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 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: f13b6d04bb
ℹ️ 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".
|
HEAD c62bbf4 incorporates #1273 at 7f45508 and addresses both current inline findings without a second signal or gRPC shutdown path. Scoped lifecycle/SIGTERM tests (including actual multi-group leadership transfer), admin forced-close tests and the complete FUSE suite pass under race. Scoped lint: 0 issues; Linux cross-compilation passes. Audited admin lifecycle and filesystem unmount callers. Work was performed in an independent temporary worktree; the user worktree was not edited. Merge order remains #1273 then #1291; #1279 is maintained separately. @codex review |
🛡️ 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. |
🛡️ 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: 8356b34e1a
ℹ️ 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".
|
Addressed the two new current-head FUSE findings: startup cancellation now reaps the ordinary helper before lazy detach, and missing mount tables retain bounded ordinary unmount without relaxing lazy-detach guards. Added regression tests; scoped lifecycle/SIGTERM/admin/unmount race tests pass (root 20.137s, FUSE 7.508s), Linux compile passes and Linux scoped lint reports 0 issues. Audited all changed release callbacks. No new merges, reaper changes or backup fixture duplication. @codex review |
🛡️ 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: 50f862525b
ℹ️ 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".
|
HEAD 324c3c1 incorporates main ac1f0d9 including #1295. The reaper cancellation guard, its regression tests and strict filesystem Jepsen harness are preserved unchanged from main. Scoped shutdown/FUSE/cleanup race tests and Linux scoped lint pass. All three PRs are also being tested together in an unpublished integration worktree. This is a signed bootjp/contact commit; no PR has been merged here. @codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? 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. |
|
Final follow-up HEAD: 2eabcc7 (signed and verified, bootjp). Independently confirmed that startup completion still preceded handoff arming by the leader-balancer setup; moved the existing arm call directly after successful server startup. The previous code-review receipt is now stale for this one-line ordering fix. Existing startup-cancel/serving-transfer/signal race tests x3 PASS (2.460s); Linux diff-filtered root lint 0 issues. All callers audited. The two older open threads were replied to and resolved only after verifying their fixes in the current source. Please review this final HEAD and new CI. @codex review |
🛡️ 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. |
|
Parent-merge integration HEAD: ac04bc7 (signed/verified bootjp). Merged latest main 5956eeb after parent merged #1273, including the inherited #1231 main changes without hand-editing them. Common shutdown/reaper semantics are preserved, and handoff remains armed immediately after successful server startup. Remaining PR diff is nine leadership-transfer/documentation/test files; no separate background-worker or ordinary-unmount implementation remains. Post-integration shutdown/signal/transfer/filesystem race x2 PASS (31.677s), Linux diff-filtered lint 0 issues. Prior CI/review on 2eabcc7 is stale for this new merge head. Please review this exact HEAD after its final push; CI is restarting. @codex review |
🛡️ 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: ac04bc7e1a
ℹ️ 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".
|
Final review-fix HEAD: c7a14b3 (signed/verified bootjp). Resolved current root 4352524502: quiesce leader balancing before handoff, using a dedicated scheduler context and a joined completion signal; Raft transport stays alive. Join and all transfers share the same five-second deadline, with bounded shutdown if balancing cannot join. Canceled startup/tick/move paths reject further balancing work. Added three shutdown regressions; audited every changed caller and updated the design doc. Final race x2 PASS (33.050s), Linux diff-filtered lint 0 issues. Parent-merged main 5956eeb is retained. Please review this exact HEAD after the final push; new CI is running. @codex review |
|
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". |
|
Current integration HEAD: 7a203e9 (signed/verified bootjp). Per parent instruction, performed a real merge of current main b086203, including #1279 Create/open and #1233 write-budget changes plus #1293 checkpoint fixes. The merge changes 26 files; no empty checkpoint commit or check bypass. Existing shutdown/balancer-quiescence semantics remain intact. Focused integration race x2 PASS: root 33.055s, kv 1.756s, store 2.735s, encryption 2.030s. Full portable FUSE race PASS (10.438s); Linux diff-filtered lint 0 issues. Native Linux real-kernel FUSE race rebuild is still running. Previous c7a14b3 receipts and its stuck managed CodeQL projection are stale for this new actual integration HEAD. Please review this exact HEAD after its final push; new CI is starting. Parent alone merges. @codex review |
|
Additional validation for unchanged HEAD 7a203e9: the complete FUSE package passed with race detection on the real Linux ARM64 kernel, with ELASTICKV_REQUIRE_FUSE=1, using disposable network-isolated cached containers. This exercises the newly merged #1279 kernel Create/open tests together with common shutdown cleanup. No source change, extra commit, push, review retrigger or check override was made for this validation. |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! 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. |
|
Final helper-race integration HEAD: aec5bf8 (signed/verified bootjp). Main run 39841698367 exposed a real race: a lookup goroutine can outlive cancellation, then default lookup reads mountHelperFallbackDir while the next test owns/writes it. The function pointer was already captured; the fallback directory was not. Fix e784b44 snapshots that default dependency before launch and skips workers for cancelled/exhausted callers, without joining uninterruptible filesystem lookup or extending the fallback budget. Real current main accae94 is merged; no separate PR or checkpoint bypass. Evidence: delayed helper-selection regression RED before fix; controlled current-main reproduction emitted WARNING: DATA RACE on the same fallback global. Fixed regression/CI participants x300 PASS; native Linux race stress x500 PASS; full portable FUSE race x2 PASS (18.785s); full real-kernel Linux FUSE race PASS. Integrated shutdown/adapter race x2 PASS (34.068s / 6.014s), Linux diff-filtered lint 0 issues. All affected callers/test hooks audited; worktree clean. Previous 7a203e9 approvals/checks are stale for this new fix HEAD. Parent alone merges. Please review this exact HEAD after its final push. @codex review |
|
Codex Review: Didn't find any major issues. Hooray! 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. |
## 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
SIGTERM leadership transfer (#1291), DCO sign-off (#1306), the filesystem and SQLite Jepsen history budgets (#1321, #1333), Jepsen artifact archives, and the review approval label series (#1328, #1332, #1336, #1340) are all on main; their remaining live-verification milestones are met by successful runs on main. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fm5mogKjCk6edosPUsnMWZ Signed-off-by: bootjp <noreply@anthropic.com>
…ns (#1349) Design: docs/design/2026_10_16_partial_test_suite_cleanup.md ## Summary A survey of every `proposed`/`partial` design doc found two kinds of unfinished work: 1. **Docs whose implementation is already on `main` but still named `proposed`.** They are promoted to `implemented` in a dedicated commit: SIGTERM leadership transfer (#1291), DCO sign-off (#1306), the filesystem/SQLite Jepsen history budgets (#1321, #1333), Jepsen artifact archives, and the review-approval-label series (#1328, #1332, #1336, #1340). Their last "verify live" milestones are met by successful runs on `main` (Jepsen Test 40657220651, review label updater 40666978702). 2. **The test-suite cleanup design.** WP-0, WP-1, WP-2 and WP-8 had already landed. This PR implements **WP-3 to WP-7** and the **WP-9 to WP-12** migrations onto the shared test-support packages, and promotes the doc to `partial`. The doc asks for one PR per WP. They are bundled here as one commit per WP (`raftengine,encryption:`, `adapter:` ×2, `main,cmd,proxy:`, `test:`), so each can be reviewed or split out separately. The larger proposed designs (serializable isolation audit, typed error boundaries, migration ownership partitions, apply-time readiness evidence, scaling/positioning roadmaps) touch replication, OCC and routing. This PR does not attempt them. ## WP-9 to WP-12 migrations All four directory sets were migrated together, one commit per pattern. The before/after counts and every intentional exception are recorded in the design doc under "WP-9 to WP-12 results": - Receive-or-fail `select` blocks: 276 candidates; 154 replaced by `testutil.Recv` / `NoRecv` / `Closed`. A bare receive becomes `Closed` only when the file closes the channel and `Recv` only when it sends on it. 122 remain, where the choice was ambiguous, the body uses `break`/`goto`, the failure is non-fatal, or hoisting a variable could shadow another name. - Legacy list delta key copies: 5 → 0 (`store.LegacyListMetaDeltaKey`, keeping each package's sequence number). - Encryption fixtures: deleted the production-exported test helper `store/encryption_test_helpers.go`; 37 `encryptiontest` call sites. - gRPC loopbacks: 14 kv servers on `grpctest.Serve`, so they now use the production server options. - Fake engines and coordinators: duplicates and pure forwarders moved onto `raftenginetest` / `kvtest`. The in-memory apply coordinators, including `fstest.MemoryDispatcher`, now dispatch through `kvtest.LocalCoordinator`, which gains a `Clock` option so a wrapper can share its HLC. Fakes whose behaviour is the subject of the test stay local (rule 4). Intentional behaviour changes from the `LocalCoordinator` move: - dispatches are serialized and elements are copied; - `ctx` is checked before applying; - the response carries `CommitTS`/`CommitIndex`; - unknown ops return `ErrUnknownOp`; - an invalid commit-timestamp patch surfaces kv's validation error instead of `filesystem.ErrInvalid`. ## Behaviour change None in production code. Production edits only delete functions that the retargets leave without callers, and fix comments that named them: `persistConfigSnapshot`, `encryptionAdminWiringForGroup`, `startPprofServer`, `startMetricsServer`, `startS3Server`, `prepareS3Server`, `readWriterRegistryLastSeen`, `extractS3Signature`, `bucketStore.invalidateQueue`/`bumpQueueEpoch`, `persistHashTxn`, `dispatchExpireForType`, `dispatchCollectionExpire`, `dispatchHLLExpire`, `dispatchStringExpire`, and the key-only Redis read-fence chain plus its discarded nil-server branch. A repository-wide grep confirms none of them has a remaining caller. ## Deviations from the plan (also recorded in the doc) 1. **WP-5 retarget 1 can't be written as specified.** `queryLeasePrefix` is only reached with an empty `IndexName`, so the hash-key-mismatch branch is unreachable. I deleted the duplicate test and added removing the dead branch as a follow-up (it is a production change). 2. **WP-6 retarget 3:** the live prepare functions moved to `internal/serverwiring`, which already tests them, so I deleted the root wrappers and their tests instead of retargeting them. 3. **WP-4 retarget 6:** `redisDispatchTimeout` is a 30 s constant. Instead of blocking, the test records the stream-deletion scan's deadline. The doc's mutation (`context.Background()` for the EXEC dispatch) survives because `txnContext.dispatchContext` bounds the context a second time. Breaking that second bound fails the test. ## Mutation table Each mutation was applied only through `go test -overlay` to a temporary copy. | WP | Mutation | Detecting test | Result | |---|---|---|---| | 3 | Remove the up-to-date fast path in `persistConfigState` | `TestPersistConfigStateSkipsSerializationWhenNewerSnapshotExists` | FAIL | | 3 | Delete `count > maxBootstrapBatchCount` | `TestBootstrap_RejectsOverCapBatchCount` | FAIL (nil error) | | 6 | Decorate the KEK unwrapper after the startup guards | `TestTimedKEKUnwrapperCoversStartupPreflightUnwraps` | FAIL ("0" not positive) | | 6 | Never close the orphan in `installOrAttach` | `TestFanoutClientForOrphanedDialClosed` | FAIL (state IDLE) | | 6 | Drop the virtual-hosted check in `newS3Server` | `TestNewS3ServerRejectsVirtualHostedStyleConfig` | FAIL | | 6 | `registryLastSeen` returns 0 | `TestRegistryLastSeen` | FAIL | | 6 | Skip address validation in `listenMonitoring` | `TestPrepareMonitoringServers` | FAIL | | 5 | Break the `=` delimiter in `findSigV4Param` | `TestExtractSigV4Signature` | FAIL | | 5 | Skip the epoch bump in `beginQueueReset` | SQS throttle tests | FAIL | | 4 | Zero `ExpireAt` in `persistHashReadTimestampTxn` | `TestRedisHDelLegacyRewritePreservesLegacyTTL` | FAIL | | 4 | Zero the TTL in `dispatchCollectionExpireReadTimestamp` | `TestRedisCollectionExpire*` | FAIL | | 4 | Encode the HLL without its expiry | `TestRedisDispatchHLLExpireWritesInlineAnchorAndScanIndex` | FAIL | | 4 | `redisLegacyBareReadFenceAllowed` always true | `TestRedisExecReadFenceUsesRedisStorageKeys` | FAIL | | 4 | Pass the uppercased name to the unsupported observer | `TestRedisMetricsUnsupportedCommandRawInvalidUTF8Bytes` | FAIL | | 4 | `hasHigherPriorityStringEncoding` returns false | `TestRedis_FastPathGuardDualEncodingReportsWrongType` | FAIL | | 4 | `txnContext.dispatchContext` returns an unbounded context | `TestRedis_StreamMultiExecDelHonoursDispatchTimeout` (both modes) | FAIL | | 4 | `context.Background()` for the EXEC dispatch (plan's mutation) | same | PASS: re-bounded by `dispatchContext` (deviation 3) | | 9..12 | `kvtest.NewLocalCoordinator` ignores `LocalOptions.Clock` | `TestLocalCoordinatorSharesInjectedClock` | FAIL | ## Test evidence - `go test -race ./internal/raftengine/... ./internal/encryption/...`: pass, except `TestFinalizeAsFSMFile_RenameFailureCleansUpSpool`. It fails identically on a clean checkout because the container runs as root, so `chmod 0o500` doesn't block writes. It is unrelated to this PR. - `go test -race . ./cmd/... ./proxy/... ./internal/serverwiring/...`: pass. - `go test -race ./internal/admin/... ./distribution/... ./keyviz/...`: pass. - `go test -race ./adapter/... .`: root passes. The adapter package passes except `TestMigrationQuorumPartitionAcceptance`, which also fails on a clean `origin/main` checkout in this container and failed once in CI on this branch (see the PR comment). `TestMigrationQuorumCommitFollowsForcedLeadershipTransfer` fails intermittently under load and passed 3/3 in isolation. Neither test touches a file changed here. - WP-9..12: `go test -race ./adapter/... ./kv/... ./store/... ./internal/filesystem/... ./internal/raftengine/... ./internal/admin/... .` pass, apart from the pre-existing failures above. `TestKernelMountReaddirPagesLargeDirectory` failed once in a five-package parallel run and then passed 3 full-package runs, 2 more parallel runs and 20 targeted repetitions under load. - `golangci-lint v2.15.0 --new-from-rev=origin/main` (the CI configuration): 0 issues. - Jepsen: not run. No runtime path changes; only unreferenced code is removed. ## Self-review (five lenses) 1. **Data loss:** No persistence path changes. The deleted functions were unreferenced copies of live paths, and those live paths now have the tests (HDEL TTL preservation, `persistConfigState` ordering). 2. **Concurrency / distributed failures:** No production concurrency changes. New tests clean up their goroutines (Redis `Run()`, recorder mutex), and the throttle tests use the production reset sequence. The `LocalCoordinator` move serializes test dispatches, as `fstest.MemoryDispatcher` already did. 3. **Performance:** No hot-path change. Only test code and dead functions changed. 4. **Data consistency:** The read-fence, EXPIRE and HDEL retargets now pin the live OCC/`ReadTimestamp` paths. The bare-key `!sqs|` exclusion is mutation-checked. The migrated coordinators keep their timestamp policies (store+1 / StartTS+1 clamp for filesystem; HLC for adapter), and the adapter wrapper shares one HLC with the dispatcher. 5. **Test coverage:** Every deletion has a named survivor, and incident/review references were moved onto the survivors. Every retarget has a mutation row. Deviations and kept fakes are listed in the doc. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01Fm5mogKjCk6edosPUsnMWZ
Design: docs/design/2026_10_22_proposed_sigterm_leadership_transfer.md
Fixes #1284
Depends on #1273. This branch incorporates its head
3eb6989351d360079d7b9e8416a52de8357a2fa0to verify the combined lifecycle. Merge #1273 first; the dependency changes will then drop out of this PR's diff againstmain.SIGTERM and SIGINT hand off every locally led Raft group before final root cancellation. Groups transfer concurrently under one five-second deadline and are re-scanned after each round, including followers that acquire leadership during another group's handoff. Targets must be active voters; learners are excluded. Unavailable successors produce a warning and shutdown continues.
Review fixes
installShutdownSignalHandler,signalShutdownsentinel, repeated-signal handling, clean-exit logic, and FUSE cleanup. During startup the callback cancels immediately;shutdownHandoff.armswitches it to a handoff request after servers and filesystem startup complete.stopGRPCServerBoundedand its ten-second drain budget. Remove the duplicate signal subscription and gRPC drain helper./bin/touch.Validation
Local validation on macOS with Go 1.26.6:
go test -race . -run '^(TestShutdown|TestInstallShutdown|TestIgnoreCanceled|TestStopGRPC|TestFilesystem)' -count=1: passed. Includes a production subprocess receiving actual SIGTERM in a three-node/two-group cluster, preserved writes, post-handoff writes, startup cancellation, repeated signals, blocked logging, clean exit, and the shared gRPC drain.-race: passed.go test -race ./internal/filesystem/fuseadapter -count=1: passed. Linux kernel-mount tests require Linux FUSE and are excluded by platform tags on macOS.golangci-lint run ./... --timeout=5mwith Go 1.26.6: zero issues.lein -o test: 204 tests, 566 assertions, zero failures/errors, including Redis workload unit tests.Before this review integration, the original head's Linux CI passed Go tests, lint, and encrypted/cleartext Jepsen. The earlier full local Go suite hit the default ten-minute timeout in existing adapter tests; the checks above cover this integrated change. The upstream Redis submodule's separate unit suite still references an absent namespace and a placeholder failing assertion; its source is unchanged. Earlier commits used a command-local hook override for the incompatible default Go/lint pairing. For this integration, the normal hook with Go 1.26.6 was blocked by another lint runner's lock; the explicit full lint run with a dedicated cache and
--allow-parallel-runnerspassed with zero issues, and the merge commit uses a command-local hook override.Operational impact
The supervisor's grace period must include the five-second handoff window, the existing listener/FUSE drain budgets from #1273, and storage cleanup. gRPC retains the common ten-second drain. A single-voter group or unavailable successor is best effort within the handoff deadline. Internal failures and memory-pressure cancellation skip the handoff and enter cleanup immediately. The separate
cmd/serverdemo is outside this production lifecycle change.Review passes