kv/kvserver: TestLeaseTransferReplicatesLocks failed

[cockroach-teamcity]

kv/kvserver.TestLeaseTransferReplicatesLocks failed on master @ 943f82fd320a79d23fc71ddb65f0223e10713081:

=== RUN   TestLeaseTransferReplicatesLocks
    test_log_scope.go:172: test logs captured to: outputs.zip/logTestLeaseTransferReplicatesLocks134941666
    test_log_scope.go:82: use -show-logs to present logs inline
    test_server_shim.go:686: DRPC is enabled (via override by TestingGlobalDRPCOption)
    client_replica_test.go:5906: transferring to s1
    client_replica_test.go:5908: done transferring to s1
    client_replica_test.go:5926: txn1: lock acquired, waiting for txn2 cancellation
    client_replica_test.go:5939: txn2: tnx1 lock acquisition observed, starting txn
    client_replica_test.go:5965: transferring lease from s1 -> s2
    client_replica_test.go:5968: cancelling txn2
    client_replica_test.go:5928: txn1: done
    client_replica_test.go:5975: 
        	Error Trace:	pkg/kv/kvserver/client_replica_test.go:5975
        	Error:      	"0" is not greater than or equal to "1"
        	Test:       	TestLeaseTransferReplicatesLocks
    panic.go:615: -- test log scope end --
test logs left over in: outputs.zip/logTestLeaseTransferReplicatesLocks134941666
--- FAIL: TestLeaseTransferReplicatesLocks (35.04s)

Parameters:

• attempt=1
• race=true
• run=1
• shard=39

Help

See also: How To Investigate a Go Test Failure (internal)

/cc @cockroachdb/kv-triage

This test on roachdash | Improve this report!

Jira issue: CRDB-60722

[wenyihu6]

/investigate

[github-actions]

Investigation: TestLeaseTransferReplicatesLocks

Investigated failure: Issue body
Failure SHA: 943f82fd320a79d23fc71ddb65f0223e10713081
Confidence: moderate

What This Test Does

TestLeaseTransferReplicatesLocks verifies that when a lease transfer occurs, unreplicated locks held in the lock table are written to storage (replicated) so that lock waiters on the new leaseholder can discover them. The test:

1. Acquires an unreplicated lock via GetForUpdate(BestEffort) (txn1)
2. Starts a second transaction (txn2) that blocks on the same lock
3. Transfers the lease from s1 to s2
4. Asserts that the leases.transfers.locks_written metric on the outgoing leaseholder (s1) is ≥ 1

Where the Failure Occurs

The assertion at client_replica_test.go:5975:

require.GreaterOrEqual(t, locksWritten, int64(1))

fails with "0" is not greater than or equal to "1". The metric leases.transfers.locks_written is 0, meaning the lease transfer evaluation found no unreplicated locks to export during OnRangeLeaseTransferEval().

Analysis

Hypothesis 1: Lock table cleared by transient lease change (moderate confidence)

The lock table on s1 was cleared between txn1's lock acquisition and the lease transfer evaluation, likely due to a transient lease loss and re-acquisition on s1.

Mechanism: Under the race detector (test took 35s), several events could cause the lock table to be cleared without the lease transfer itself failing:

1. Raft leadership change: With leader leases enabled by default (replica_range_lease.go:95-104), a temporary Raft election timeout under race could cause s1 to lose and re-acquire its lease. When the lease is lost, OnRangeLeaseUpdated calls m.lt.Clear(true) (concurrency_manager.go:720-723), clearing all unreplicated locks and disabling the lock table. When the lease is re-acquired, the lock table is re-enabled but empty. If the lease transfer is then initiated with the new (higher) sequence, the evaluation finds no locks.

2. Snapshot application: If a Raft snapshot is applied to s1's replica, OnReplicaSnapshotApplied calls m.lt.Clear(false) (concurrency_manager.go:760-770), which clears all locks without disabling the table.

3. Background range split with metamorphic setting: UnreplicatedLockReliabilitySplit is metamorphically set (concurrency_manager.go:137). If this is false and a background split occurs on the scratch range, OnRangeSplit calls m.lt.Clear(false) (concurrency_manager.go:738-739), clearing all locks.

Evidence:

• Test ran under race=true, taking 35 seconds (significant slowdown)
• The lease transfer succeeded (no error at line 5966), so the lease was valid at evaluation time
• The only way the metric can be 0 is if ExportUnreplicatedLocks (lock_table.go:4938-4994) found no unreplicated locks, either because the table was disabled (!t.enabled) or because the lock was no longer present
• The test does not check that the lock is still in the lock table before the lease transfer

The most likely sub-scenario is option 1 (Raft leadership change / lease re-acquisition), because:

• Under race, Raft heartbeats can be delayed
• Leader leases are directly tied to Raft leadership
• A leadership change would cause the lease to be lost and re-acquired with a new sequence, clearing the lock table each time
• The AdminTransferLease call at line 5966 would use the new (current) lease as prevLease, so the transfer would succeed even after the lease change

Hypothesis 2: Lock marked ineligible for export (low confidence)

The lock could have been marked ineligibleForExport via OnLockMissing if a QueryIntent observed the lock as missing. This seems unlikely in this test scenario since there's no explicit QueryIntent involved.

Recommended Fix

The test should be made more robust to handle transient lease changes. Options include:

1. Use testutils.SucceedsSoon to retry the metric check, as the lock might need to be re-acquired after a transient lease change.
2. Verify lock presence before transferring: Add a check (e.g., via the lock table's test-only string output) that the lock is still present before initiating the lease transfer.
3. Use ScratchRangeWithExpirationLease to avoid leader lease dynamics, or explicitly disable leader leases in the test.
4. Re-acquire the lock if the lease changed: After the initial TransferRangeLease to s1 and before starting txn1, verify the lease sequence and ensure it hasn't changed by the time the test initiates the second transfer.

Related Issues and PRs

• kv/kvserver: TestLeaseTransferReplicatesLocks failed #164262 (this issue) — TestLeaseTransferReplicatesLocks failed (open, release-blocker)
• PR f26bec4 — concurrency: flush unreplicated locks on lease transfer (original test creation by Steven Danna, 2025-01-29)
• PR 58bf21e — kvserver: deflake TestLeaseTransferReplicatesLocks (prior deflake for channel double-close)
• kvserver: consistently apply strict follower-behind checks to lease transfers #163267 — kvserver: consistently apply strict follower-behind checks to lease transfers (open, related to lease transfer safety)
• kv: implement replicated key-level locks #100193 — kv: implement replicated key-level locks (closed, major feature this test validates)

Timeline

• 2025-01-29: Test created in f26bec4b3d (concurrency: flush unreplicated locks on lease transfer)
• 2025-03-24: Prior deflake in 58bf21e9a1 (fixed channel double-close on txn retry)
• 2026-02-19: 74d3492eb2 enabled VIR metamorphically (44 commits before failure SHA)
• 2026-02-24: Failure at SHA 943f82fd32 under race=true

Detailed evidence and investigation notes

Log Excerpts

Test output shows normal execution flow:

client_replica_test.go:5906: transferring to s1
client_replica_test.go:5908: done transferring to s1
client_replica_test.go:5926: txn1: lock acquired, waiting for txn2 cancellation
client_replica_test.go:5939: txn2: tnx1 lock acquisition observed, starting txn
client_replica_test.go:5965: transferring lease from s1 -> s2
client_replica_test.go:5968: cancelling txn2
client_replica_test.go:5928: txn1: done
client_replica_test.go:5975: Error: "0" is not greater than or equal to "1"
--- FAIL: TestLeaseTransferReplicatesLocks (35.04s)

The 35-second runtime (vs typical sub-second for such tests without race) confirms significant race detector overhead.

Code References

Key code paths involved:

• Lock export during lease transfer: cmd_lease.go:73-93 — calls OnRangeLeaseTransferEval() which calls exportUnreplicatedLocks()
• Export function: lock_table.go:4938-4994 — iterates lock table, skips if disabled or no unreplicated locks found
• Lock table Clear on lease loss: concurrency_manager.go:714-724 — disables and clears lock table when leaseholder status is lost
• Metric increment: metrics.go:4964 — increments LeaseTransferLocksWritten counter from local result metrics

Things Ruled Out

• Metric on wrong store: The test checks server 0 (s1), which is the outgoing leaseholder and proposer of the transfer. The metric is applied via handleReadWriteLocalEvalResult on the proposing replica. This is correct.
• Lock not acquired by txn1: The GetForUpdate(BestEffort) path correctly creates an unreplicated lock acquisition (cmd_get.go:128-134) and handleReadOnlyLocalEvalResult adds it to the lock table (replica_read.go:616-622).
• VIR interference: Virtualized intent resolution (metamorphically enabled at 74d3492eb2) applies to intent resolution, not to unreplicated lock management during lease transfers. Unlikely to be related.
• UnreplicatedLockReliabilityLeaseTransfer disabled: The test explicitly overrides this to true at line 5888.
• Lock table size limits: Default is 10,000 locks; the test only has 1 lock.
• Timing of txn2Started signal: txn2StartedOnce() fires before txn2's GetForUpdate, but txn1's lock is already in the lock table by this point (confirmed by the code path analysis). The lease transfer acquires global latches which synchronize with any concurrent requests.

Other Failure Occurrences

This is the first reported occurrence of this failure. The test was previously deflaked once (58bf21e) for a different issue (channel double-close on txn retry).

Tooling Feedback

• No artifacts were available for this EngFlow-based failure (no TeamCity build ID). The EngFlow URL returned a 303 redirect that could not be followed. Having downloadable test logs from EngFlow would have been valuable to check for lease change log messages during the test.
• Access to the CockroachDB node logs from the test (via the test_log_scope) would have been very helpful to confirm whether a lease change occurred during the test.

---

Investigation triggered by this workflow run.