fix(lockservice): bind CN drain completion to service instance

[VioletQwQ-0]

What type of PR is this?

• API-change
• BUG
• Improvement
• Documentation
• Feature
• Test and CI
• Code Refactoring

Which issue(s) this PR fixes:

Kernel-side work for #29477; keep the issue open for Operator/cloud acceptance.
Related: matrixorigin/matrixone-operator#623. Companion: matrixone-operator#624.

What this PR does / why we need it:

A CN with no client sessions can still serve locks held by transactions on another CN. An allocator lookup miss, a UUID matching a different incarnation, or a stale restart response must not authorize eviction. BeginDrain/QueryDrain bind completion to the exact lock-service incarnation, drain attempt, and allocator identity/epoch. Missing or mismatched proof fails closed; completion requires the CN's existing local drain checks and terminal allocator observation.

The protocol preserves existing admission and cleanup ownership. Invalid allocator bindings are rejected before publication, allocation waiters are released on failure, and retries cannot reopen drain admission. After allocator state loss, a pending non-admitting binding requires a matching-epoch heartbeat; an already-draining CN can recover without returning to Enable. Terminal heartbeats containing active transactions remain rejected. Retired proofs are bounded and retries are idempotent. Transient timeout validation failures no longer leave an idle draining CN stuck behind negative retirement: BeginDrain reuses the pending non-admitting handshake, clears only obsolete retirement metadata, and preserves inactive-service and commit fences. Positive retirement proofs remain idempotent. IdentityOnly fetches the exact identity without enumerating locks; existing requests retain their lock-list behavior.

Rebase and deployment contract

Rebased onto main f1137a6cac72852be2ab2e5078b684b98f23d96c. The nine ordinary commits were replayed separately; merge-only recovery documentation and protocol guards were preserved explicitly. Drain RPC methods remain 23/24 and now require MORPC v105, preserving main's v102 vector-cache control, v103 distributed IVF PRE, and v104 typed JSON behavior. Generated bindings preserve upstream query cache fields, LockOptions.KeepRows, and the removed Result trace fields' reserved numbers 6–9. Generation uses the repository's lock gogofast/query gogofaster modes and is reproducible byte for byte.

Acceptance is a coordinated downtime upgrade: stop CN/TN services, deploy compatible binaries, restart them, then enable the matching Operator client. Mixed-version and rolling upgrades are outside this release's acceptance scope. Version rejection and fail-closed proof handling still apply. This PR does not update the companion Operator's dependency pin; integration must use the rebased kernel revision.

Prior rebase validation at 3856a3dae1

• Fresh native make cgo build passed.

• Full normal tests passed for lockservice (90.934s), cnservice (9.776s), lockop (5.334s), pb/query, and defines, with -short -tags matrixone_test -count=1 -vet=all.

• pb/lock has no package tests; it compiled successfully and its schema behavior passed consumer tests and the external wire QA.

• Focused real RPC/instance-bound proof, allocator-loss recovery, stale admission, and remote-holder tests passed; capability tests reject versions 99–104 and accept v105.

• Six-package go vet and repository-config golangci-lint, make err-check, and git diff --check passed.

• External wire QA passed: existing vector-cache request fields coexist with IdentityOnly; old trace tags decode safely; reserved fields and KeepRows survive generation.

• Full lockservice race tests passed (97.786s). Both changed capability tests also passed 100 repetitions each under race.

• Separately configured gpt-6.1-sol / xhigh design and final overall review passed. Final reviewed revision: 3856a3dae1eaf7ff4d644d48d7ed5cdfc632b3d2; no unresolved material blocker. Review reconciled complete ownership/state/wire/test closures and independently checked original-history and upstream preservation.

Negative-retirement recovery at b559459d73

The original allocator implementation fails the new real two-CN negative-retirement regression; allocator-loss is the passing control. The correction changes only BeginDrain recovery and adds no normal lock-path work, worker, or retry loop. Existing recovery tests now cover both failure modes, including held remote locks, stale heartbeats, non-admission, old-generation cleanup, retained named/persistent commit fences, and terminal positive-proof retries. A duplicate phase test and ordinary SQL atomicity cases that never invoked drain were removed; actual drain coverage remains in real RPC/two-CN tests.

• Fresh native build and full lockservice normal tests passed (89.977s); final strengthened recovery matrix passed (0.275s).
• Full lockservice race passed (90.581s). Recovery handshake and real remote-holder recovery each passed 100 exact race repetitions (7.823s / 22.807s).
• Final go vet, repository-config golangci-lint v2.6.2, make err-check, and diff checks passed.
• Separately configured gpt-6.1-sol / xhigh design and final overall review passed at b559459d734c0a0e2c38fb2ed5b7dbc2ae399014; no unresolved material blocker.

CI upgrade-test fixture correction at 713f4bd3de

The previous CI run passed SCA, shared build, and multi-CN BVT, but failed TestV408LoginRejectsAccountDroppedAfterAuthentication: DROP ACCOUNT legitimately closed the borrowed authenticated session, and the test called MaybeUpgradeTenant after its owner had destroyed it. The test and frontend code were unchanged from the frozen PR base. This correction retains real AuthenticateUser, captures authenticated scalar values before DROP, and checks compensation twice through its stable real CN owner, CheckTenantUpgrade. Existing wrapper UT covers version gating/error propagation. Both ErrNotFound results, no successful-cache publication, and independent CN SQL liveness remain required. Query-registry removal before tenant mutation remains; account routine cleanup is unchanged. No production nil guard, retry, sleep, hook, or normal-query overhead is added.

• The three related V408 integration tests passed normally (10.622s); the previously failing account-drop test passed under race (22.308s).
• Incremental upgrade-package vet/lint (0 issues), make err-check, and diff checks passed.
• Separately configured gpt-6.1-sol / xhigh design and final overall review passed at 713f4bd3de8f58eb24cbd6172a2b28b37906ba9c. Drain implementation and its prior acceptance evidence remain unchanged.
• Initial local startup attempts encountered an overlong Unix socket path; only the external test wrapper TMPDIR was shortened before the successful source-bound runs. No production change was made for that environment error.

The current evidence is local Linux kernel validation. Live SQL distributed cases, real process-restart deployment, Operator/Kruise eviction, and cloud control-plane acceptance were not rerun for this rebase. They remain separate integration gates; earlier experiments on previous heads are not presented as current-head proof. GitHub CI will run on the pushed revision and is not claimed as passed.

[VioletQwQ-0]

@iamlinjunhong Please re-review your P1/P2 findings at b8a96a9da3.

P1 — stale admission and publication: an already-draining CN still returns ErrNewTxnInCNRollingRestart at admission. If admission succeeded but drain/retirement invalidates allocation, the handler now returns ErrLockTableBindChanged for the existing bounded retry path. Client validation rejects invalid/ownerless/wrong-group/wrong-table responses and still accepts valid remote owners. TestGetBindDrainInterleaving pauses the real handler between canGetBind and Get and covers new/existing binds, legacy restart, retirement and a no-drain control. The same oracle on c742b6a reproduces successful empty binding in all four negative cells. TestGetBindInvalidResponseReleasesAllocation proves no cache pollution, removal/closure of allocation state, waiter exit and successful later publication. Race repetition passes at 75 and 100 runs respectively.

P2 — canonical client: TestInstanceBoundDrainCanonicalClient uses actual NewClient.Send and allocator handlers. v99 rejects both methods before server dispatch; v100 traverses the transport and checks the complete proof, incomplete/completed drain, wrong proof fields and cancellation. It passes 100 race repetitions. MORPC v100/method IDs 23/24 remain unchanged; no legacy fallback is added. Single-process version switching is not described as mixed-binary testing.

Validation uses the repository mo-cgo-test wrapper and matching native dependencies (Go1.26.4, macOS arm64). All final regression selections pass; the owning lockservice package passes normal/coverage and race (646 tests), cnservice passes 135 tests, and lockop passes 139 tests in CI-equivalent short/tag mode. Final standard vet, formatting, diff check and incremental static checks are recorded in the PR body. One unchanged wall-clock-sensitive test failed the initial package race run under compilation load; its 1-second expiry explains the failure, the isolated pre-repair overlay passes, and the final whole-package race passes without changing assertions. The initial failure is retained in the evidence.

The PR remains Ready. Final-head CI and your explicit re-review remain gates; this update does not claim Operator integration, mixed binaries, Kubernetes/Kruise eviction or cloud acceptance.

[VioletQwQ-0]

@XuPeng-SH Please re-review your P1/P2 findings at b8a96a9da3.

P1 — stale admission contract and terminal behavior: TestGetBindDrainInterleaving now controls the real handler with channel barriers, including an existing binding and safe retirement. No late bind is allocated, no retired service is resurrected and an established proof remains safe. Old c742b6a fails the same oracle with error=nil/Valid=false/empty owner/table=0. Client tests independently vary Valid, owner, Group and Table; local/remote owners and ByRow are positive controls. The service case verifies allocation cleanup and recovery. TestLockWithRetryStopsAfterBindChangedThenDrainRejection and TestLockWithRetryBindChangeHonorsCancellation, together with existing success/budget tests, prove that the new bind-change error uses bounded retry and the next drain rejection terminates. No frontend rollback policy or global retry layer changed.

P2 — usable negotiated API: the real canonical-client test proves the v99 rejection/v100 server round-trip, matching proof, incomplete/completed state and mismatched proof rejection. TestGetLockServiceIdentityDoesNotEnumerateLocks additionally proves identity-only does not iterate and a legacy request returns an actual held row with table/key/mode/holder. We do not claim the Operator has enabled IdentityOnly.

The Valid=true fixture corrections reviewed by aptend are retained; both focused refreshed-binding tests and the owning package/coverage checks pass.

Validation uses the repository mo-cgo-test wrapper and matching native dependencies (Go1.26.4, macOS arm64). All final regression selections pass; the owning lockservice package passes normal/coverage and race (646 tests), cnservice passes 135 tests, and lockop passes 139 tests in CI-equivalent short/tag mode. Final standard vet, formatting, diff check and incremental static checks are recorded in the PR body. One unchanged wall-clock-sensitive test failed the initial package race run under compilation load; its 1-second expiry explains the failure, the isolated pre-repair overlay passes, and the final whole-package race passes without changing assertions. The initial failure is retained in the evidence.

The PR remains Ready. Final-head CI and your explicit re-review remain gates; this update does not claim Operator integration, mixed binaries, Kubernetes/Kruise eviction or cloud acceptance.

[XuPeng-SH]

Design-focused deep re-review of b9013dc6bb058ec74935a62ac3a35f57d13c2f98..b8a96a9da3c717a9d85cf4a9d9f8815fdbbc7b5c. No subagents were used.

REQUEST_CHANGES: the original kernel defects are repaired, but the allocator-loss recovery contract remains open. This continues the recovery concern in the earlier design review; it is not another report of the old zero-binding bug.

Remaining design blocker

A CN may enter Waiting and then lose its allocator's state before completion is durably observed. Its old proof is correctly rejected by the new allocator epoch. However, asking the new allocator for a fresh attempt creates a pending bind that accepts only a ServiceLockEnable heartbeat. The same live CN continues sending Waiting, UnLockSucc or CanRestart; the real heartbeat handler rejects those before clearing drainAwaitingHeartbeat. CN keepalive failure purges/fences bindings but does not return the CN to Enable. Repeated BeginDrain/QueryDrain therefore cannot recover this instance even after its transactions finish.

I independently exercised the fresh-allocator boundary through the real heartbeat handler. With identical instance/attempt setup, the Enable control passes; Waiting, UnLockSucc and CanRestart each keep heartbeat OK=false and awaiting=true through three heartbeats, and the expected fresh-attempt acceptance fails. This is a deterministic component witness plus CN caller tracing, not a process-restart/Kruise experiment. It confirms the missing recovery transition, not an unsafe success response. The existing cold-CN test deliberately rejecting stale completion is a useful safety control and should remain.

Please define and validate an executable safe recovery sequence: a fresh, instance-bound handshake that works for a still-draining CN, or a concrete operational recovery procedure with its independent fencing/completion evidence. Simply changing the attempt ID does not work. Do not accept an old completion heartbeat without a fresh proof or reopen normal admission to manufacture Enable.

This change adds wire methods/versioning and a cross-CN/TN/Operator lifecycle protocol. I did not find a stable, versioned design with a traceable approval in either PR. Record the final state transitions, allocator-loss recovery, compatibility/rollout order and validation ownership in one final contract document. Under the design-first review gate, implementation approval remains blocked here; this review does not claim an exhaustive generated-code/rollout approval.

Historical comments

• Original P1 closed: late admission now returns ErrLockTableBindChanged; the client validates Valid/owner/group/table before publication. The deterministic new/existing/legacy/retirement/control cells and allocation-waiter cleanup test pass.
• Original P2 closed: canonical client capability checks register BeginDrain/QueryDrain at v100; real transport tests prove v99 rejection and v100 proof round trips.
• aptend's fixture blocker closed: the two changed-binding mocks retain Valid=true. Current-head required CI, UT, coverage and BVT pass.
• Identity-probe cost addressed: IdentityOnly returns before IterLocks; its test also checks a real legacy lock entry.
• Recovery and release acceptance remain open: fresh proof after allocator loss, mixed binaries and real Operator/Kruise eviction are not supplied by the repaired kernel unit tests. The companion PR's timeout repair is outside this kernel review's verified implementation scope.

Ownership, liveness and cost

Audit	Result
Q1 ownership	Rejected binding responses are released; allocating entries/channels terminate without publishing an invalid route.
Q2 termination	Stale-bind retry/drain rejection and cancellation are covered; allocator-loss drain recovery is the outstanding state-transition problem above.
Q3 growth	Retirement records have a 24-hour default cleanup horizon; instance identity lookup avoids lock-count-dependent enumeration. No new worker is added.

Fresh local CGo validation on macOS arm64/Go 1.26.4: the five focused handler/client/publication tests pass under race detection (package 2.927s). The separate recovery challenge has one passing control and three expected failing recovery cells (package 0.573s). Native inputs match the provenance-verified artifact source. I also checked the author-reported final-content coverage/race evidence and live current-head CI; green checks do not establish the missing recovery contract.

QA sources and logs are retained under /private/tmp/mo-review-29291-29425.q67o1N/. PR production code and tests were not changed.

[aptend]

Deep re-review at exact head b8a96a9da3c717a9d85cf4a9d9f8815fdbbc7b5c (full PR diff plus the increment since my prior approval, historical reviews/replies and the unresolved thread). The earlier zero-bind and client-version defects, and the stale test fixture, are closed; required CI is green. I independently traced one remaining blocking liveness path.

[P1] pkg/lockservice/lock_table_allocator.go:466-478,1407-1418,1566-1581: if a CN has already received Waiting from allocator A, then A restarts before completion, allocator B has no bind for that exact service. BeginDrain on B creates drainAwaitingHeartbeat=true and returns OK=false. The CN's actual lock_table_keeper.go:544-550 heartbeat reports its local Waiting/UnLockSucc/CanRestart, never Enable; confirmPendingDrainHeartbeat rejects each, so B never clears the pending flag. service.go:2594-2635 purges stale bindings on OK=false but does not reset the CN's drain status. Thus even after remote transactions release, repeated BeginDrain/QueryDrain on B cannot yield a fresh safe proof; a protected CN cannot be evicted without an out-of-band, independently verified recovery. This is a concrete allocator-restart interleaving, not a repeat of the repaired GetBind race. The current cold-CN test's Enable control does not cover a previously draining CN.

Please add a safe instance-bound re-handshake or specify and validate a concrete operational recovery/fencing procedure, with a regression that runs the old-allocator Waiting state into a fresh allocator and verifies eventual completion after transactions drain. Do not accept stale completion heartbeats as proof. I also confirmed the current v100 method negotiation, identity-only fast path and bounded bind-change retry; no separate blocker there. The separate Operator rollout/pod-eviction acceptance remains open.

[VioletQwQ-0]

@aptend @XuPeng-SH The allocator-loss recovery blocker is addressed at 7c3a8d7. A CN already in Waiting/UnLockSucc/CanRestart re-handshakes with the fresh allocator by echoing its new ID/version on a later heartbeat; the pending bind remains non-admitting until that exact-epoch heartbeat. Terminal heartbeats carrying transactions are rejected, and the real two-lockservice held-remote-lock regression remains unsafe until Unlock. The final state/rollout contract is documented in docs/design/20260928-cn-safe-drain-allocator-recovery.md. Final-head lockservice normal/race, cnservice and lockop tests passed with a matching native build; full CI is rerunning on the pushed head. This is still not a real process-restart or mixed-binary/cloud eviction claim. Could you re-review the transition and remaining release gates?

[XuPeng-SH]

Reviewed 7c3a8d72efd10b0689b89bae2cb3b3c5f08a39fc against merge-base afa347996b878336ff294f5cd070492194352b30, including all 22 changed files, historical reviews and the allocator-loss design contract. No subagents were used.

APPROVE for the kernel change. The previous allocator-loss recovery finding is closed; this is not cloud/Operator release approval.

Design and correctness

The identity contract is coherent: exact lock-service incarnation + attempt + allocator identity/epoch, with no UUID/lookup-miss fallback. BeginDrain atomically closes allocator bind admission; a cold/replacement allocator creates a pending non-admitting record. Requiring the CN to observe and echo the replacement epoch allows Waiting/UnLockSucc/CanRestart recovery without reopening Enable. The CN's existing local completion checks remain authoritative. An epoch echo establishes observation, not completion: Waiting and terminal heartbeats carrying transactions remain unsafe.

I traced bind publication, keeper observation/fencing, terminal status, timeout cleanup and retirement reuse. The GetBind admission/Get interleaving is rechecked under allocator ownership; invalid responses cannot enter the client's bind cache, and allocation waiters are released on failure. Same-attempt retries and delayed phases are monotonic; exact retirement records cannot authorize another incarnation. Retirement cleanup bounds retention by the configured disconnect duration; expiration loses the proof and fails closed.

This is a justified protocol change rather than another heuristic based on session/lock counts. Normal GetBind adds constant-time identity/status validation, not a lock-table scan. IdentityOnly exits before IterLocks, avoiding copying an entire busy CN's lock list. No new worker or unbounded retry loop is introduced. Existing aliases are compatibility-only and do not provide the new safety contract.

Historical comment closure

• Admission revocation returning an invalid successful bind: fixed on both allocator and client boundaries, including waiter cleanup and the next terminal drain rejection.
• Canonical RPC capability registration/version collision: methods 23/24 require v101; v99/v100 reject before transport; main's v100 assignment is preserved.
• Refreshed-binding fixtures: retain Valid=true; no weakened production validation.
• Identity query cost: real-lock legacy control retained; IdentityOnly does not enumerate.
• Allocator-loss dead end: resolved by the fresh-epoch handshake and real remote-holder test, not by resetting the CN to Enable.

Validation and limits

Fresh reviewer validation on this head: focused drain/identity/admission/bind tests PASS normally (3.330s) and under race detection (4.181s). A separate reviewer-owned handler challenge combines three successive allocator-state losses with different attempts, stale terminal proofs, active-transaction terminal rejection, continued bind rejection, and eventual completion. It passed 20 repetitions under race detection (4.587s). Both generated protobuf files reproduce exactly with protoc 3.21.1 and the matching gogo generators/import mapping. Diff check passes.

I reused the author's exact-head owning lockservice normal/race package evidence and cnservice/lockop results, rather than claiming to have rerun those full packages. Current CI Required is SUCCESS. Local checks used Go 1.26.4 darwin/arm64 CPU CGo artifacts with verified provenance; native CPU source inputs are unchanged by the intervening GPU-toolchain changes.

Actual allocator process restart, mixed CN/TN/Operator binaries, Pod/container incarnation binding, timeout protection and all Kruise/cloud eviction routes remain the explicitly documented release gates owned by the companion integration. The in-process state-loss test is not those tests, and the supplementary SQL case does not exercise eviction. Do not close operator #623 or claim seamless/no-retry business migration from this approval.

Local reviewer evidence: /private/tmp/mo-review-drain-drop-OKsx8n/. No PR source, branch, CI or review-thread resolution state was modified by this review.

[aptend]

Deep re-review at exact head 7c3a8d7. I read all historical reviews, author replies, and the resolved/outdated allocator-loss thread; compared the repair after my b8a96a9 review with the full PR diff; and traced the allocator, CN keeper, canonical RPC/version checks, GetBind publication, tombstone cleanup and exact-instance/attempt/epoch proof. The old invalid-binding admission race, version negotiation gap, stale test fixtures, and previously blocking allocator-loss dead end are closed. In the fresh-allocator path, pending bind admission remains disabled until an exact-epoch heartbeat, and a Waiting heartbeat does not imply Safe; terminal reports with active TxnIDs are rejected. The new design contract at docs/design/20260928-cn-safe-drain-allocator-recovery.md (blob 7dc53f708241244a93c6041e059d8c6ebd4dfb21) is traceably reviewed and accepted at this same head in review 5341334387; the kernel implementation aligns with it. The exact-head lockservice normal/race, CN, lockop and CI evidence is green; diff check passes. No remaining blocker in this kernel PR. This is not an approval of real allocator-process restart, mixed-version rollout, Operator/Kruise/cloud eviction, timeout recovery, or incident closure: those documented release gates still require their own evidence.

[fengttt]

Where is the PR link?

[VioletQwQ-0]

@fengttt The companion Operator PR is matrixorigin/matrixone-operator#624. I added this explicit link and the MatrixOne tracking issue #29477 under the PR issue section. This kernel PR does not close #29477: Operator/cloud eviction and QA remain separate validation gates. Could you please re-review this clarification?

[aunjgr]

Reviewed 7c3a8d7 as a kernel-only drain protocol. The allocator binds BeginDrain/QueryDrain to exact service incarnation, attempt, allocator identity/epoch; missing or ambiguous state fails closed, and re-handshake after allocator loss remains pending until the CN echoes the new epoch. GetBind publication is rechecked after admission, and retirement proof is bounded and not inherited by another incarnation. The focused protocol/race and two-CN remote-holder tests plus green CI support this kernel scope. I found no concrete remaining kernel blocker. This approval does not certify the separate Operator/cloud eviction rollout or mixed-binary deployment gate.

[XuPeng-SH]

Confirmed P2: a transient ValidateService connection failure can retire the allocator bind of a CN that is already draining. After connectivity returns and its remote transaction releases, this exact-attempt drain still cannot recover. Normal and race white-box probes reproduced the failure; the healthy control passes. Please restore a safe pending handshake for negative/unknown retirement state while preserving fail-closed QueryDrain and preventing new lock admission. Full trigger, evidence, and constraints are in the inline comment.

[XuPeng-SH]

Fixed the latest UT CI failure in 713f4bd.

The failing test borrowed an authenticated frontend session. DROP ACCOUNT legitimately killed its routine and destroyed the session, then the test called MaybeUpgradeTenant on that destroyed object. The CI log records the account/connection kill and cleanup before the nil dereference. The correction preserves actual AuthenticateUser and pre-DROP scalar/version assertions, removes query-registry exposure before tenant mutation, and uses the stable real CN CheckTenantUpgrade owner for two post-DROP checks. Both ErrNotFound results, failed-check cache behavior, and independent SQL liveness remain required. No production change or test lifecycle hook was added.

The three related V408 integration tests passed normally (10.622s); the exact failing case passed under race (22.308s). Incremental vet/lint (0 issues), make err-check, and diff checks passed. Separately configured gpt-6.1-sol / xhigh design and final overall review PASS at this exact revision. Prior drain evidence remains valid; new-head CI is pending and not awaited.

[XuPeng-SH]

Deep review of PR #29291 at 713f4bd3de8f58eb24cbd6172a2b28b37906ba9c, against f1137a6cac72852be2ab2e5078b684b98f23d96c. Working tree clean. Primary-agent review; no subagents used.

APPROVE for the kernel change. No new material correctness, liveness, or design blocker found. This supersedes my negative-retirement REQUEST_CHANGES after verifying its correction and surrounding lifecycle. Required CI and Operator/cloud acceptance remain separate gates.

One non-blocking test-quality item: TestSetRestartServiceIsIdempotentAfterSafeCompletion in pkg/lockservice/lock_table_allocator_test.go:245 repeats the ServiceCanRestart cell of TestDrainDuplicateRequestEveryPhase in drain_identity_regression_test.go:78. Its extra bound table has no independent assertion. Fold it into the existing matrix, or retain that setup only with a distinct binding-preservation oracle. This is a small cleanup opportunity, not a reason to add another fixture or abstraction.

The complete change was reviewed by responsibility, including the generated contract and independently counted increments:

Closure	Increment (+/-)	Review result
Handwritten implementation and protocol definitions	417 / 33	Exact incarnation/attempt/epoch proof extends the existing allocator owner; CN local admission, references, in-flight operations and transaction holders remain the completion authority.
Tests	1271 / 12	Real client/version, stale allocation publication/waiter cleanup, identity-only versus actual lock listing, recovery, expiry, and remote-holder cases have distinct oracles. One duplicate terminal restart cell remains as noted above.
Generated bindings	2208 / 513	Source schemas and generated files are unchanged from the validated rebase at 3856a3d; previous reproducible-generation/wire evidence remains valid. New method IDs 23/24 and v105 preserve upstream v102-v104 features and existing tags.
Design document	100 / 0	Describes allocator-loss and negative-retirement recovery, same-epoch freshness limits, downtime rollout, and separate acceptance owners.

The new branch is justified: absence after transient validation failure must not permanently strand a live draining incarnation. BeginDrain clears obsolete negative retirement metadata and uses the existing pending, non-admitting handshake. It preserves inactive-service and commit fences. Matching an allocator epoch establishes observation; it is not by itself a completion proof. Same-incarnation terminal state is irreversible, and terminal reports containing transactions are rejected. Positive retirement remains exact-attempt idempotent. The new allocator mutex boundaries prevent a GetBind admitted earlier from publishing a now-invalid route or resurrecting a completed drain.

Unhappy-path audit	Closure
Q1 ownership	Allocator owns bindings/proofs; old-object retirement checks identity before removal. Invalid responses never enter the CN holder cache; allocating channels are removed/closed and request/response resources released. Upgrade-test session remains routine-owned; post-DROP checks use the stable CN owner and pre-DROP scalar snapshots.
Q2 termination	Bind-change retries preserve existing cancellation/budget and terminate on drain rejection. Recovery admits neither a second attempt nor new normal binds; held remote transactions keep completion false. Existing mutex order is retained; no new worker or blocking retry loop.
Q3 growth	Retired proof metadata uses the existing periodic cleanup and default 24-hour horizon. Pending bindings use existing timeout handling. No additional per-lock/request history is retained.

Historical kernel comments are addressed: stale GetBind success/publication, canonical method-version registration, refreshed Valid=true fixtures, identity-only enumeration cost, draining-CN recovery after allocator loss, and negative-retirement recovery. The latest upgrade CI fixture panic is also repaired. Operator timeout/epoch recovery, real Pod/container identity validation and controlled Kruise eviction are not certified by this kernel review. Coordinated downtime is the explicitly accepted upgrade contract; mixed-version deployment is outside it.

Performance: identity-only lookup avoids work proportional to local lock count. The new recovery branch is control-path work; normal GetBind adds constant validation. The common non-nullable lock RPC envelope does have a fixed +6 bytes per ordinary request and response, plus larger pooled structs; previously measured serialization cost was about 18-20ns per small message. That microbenchmark is not an end-to-end SQL/TPCC regression estimate. No new O(row count) work, extra network round trip on successful normal locking, or background state machine was introduced.

Evidence reused after checking relevant source/fixture identity: full lockservice normal/race (89.977s/90.581s); two recovery tests each 100 exact race repetitions; three V408 integration controls normal (10.622s) and the corrected account-drop case race (22.308s); incremental vet/lint and repository err-check. Existing CN/lockop/protobuf consumer evidence remains valid because those inputs are unchanged. The initial local overlong Unix socket-path failure occurred before test execution and is not presented as a product regression or successful test.

New independent QA used a test-only overlay, without changing PR source: concurrently submit two attempts after generation-checked negative retirement, then deliver an old-object retirement and real RPC heartbeats. Both initial Begin responses stay pending; exactly one attempt becomes accepted; neither is safe with a remote-holder report; only the accepted attempt obtains the terminal/retired proof; the independent inactive fence remains. It passed once under race (0.04s body), then 100 exact repetitions under the bounded 30s budget (5.341s package). Artifacts: concurrent_attempt_test.go, overlay.json, concurrent-once.log, concurrent-100.log under /home/xupeng/.codex/reviews/matrixone/pr29291-713f4bd3-deep/.

The CI snapshot at review time has a successful shared build and UT coverage; UT/SCA/multi-CN checks were still running. This review does not claim those pending checks or Operator/cloud deployment acceptance passed.