Skip to content

Commit 5fb9f4e

Browse files
pav-kvclaude
andcommitted
kvcoord: deflake TestTxnCoordSenderRetriesAcrossEndTxn
The first subtest asserted ExpectPusheeTxnRecordNotFound, which requires the push to find no txn record at all. However, the heartbeat loop can race and write a PENDING txn record during CommitInBatch execution, causing the push to find that record and fail the assertion. Replace with the new ExpectNoTxnRecovery expectation, which verifies the actual invariant: no STAGING record was found and no transaction recovery was performed. This tolerates a PENDING record from the heartbeat loop while still checking that the txn was not implicitly committed. Fixes: #170829 Release note: None Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 980b3b2 commit 5fb9f4e

2 files changed

Lines changed: 17 additions & 3 deletions

File tree

pkg/kv/kvclient/kvcoord/dist_sender_server_test.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3962,9 +3962,11 @@ func TestTxnCoordSenderRetriesAcrossEndTxn(t *testing.T) {
39623962
sideRejectedOnSecondAttempt: left,
39633963
// The first attempt of right side contains a parallel commit (i.e. an
39643964
// EndTxn), but fails. The 2nd attempt of the right side will no longer
3965-
// contain an EndTxn, as explained above. So we expect the txn record to
3966-
// not exist.
3967-
txnRecExpectation: kvclientutils.ExpectPusheeTxnRecordNotFound,
3965+
// contain an EndTxn, as explained above. So we expect no STAGING txn
3966+
// record / no transaction recovery. Note that a PENDING txn record may
3967+
// or may not exist depending on whether the heartbeat loop managed to
3968+
// fire before the CommitInBatch completed.
3969+
txnRecExpectation: kvclientutils.ExpectNoTxnRecovery,
39683970
},
39693971
{
39703972
// On the first attempt, the right side succeed in writing a STAGING txn

pkg/testutils/kvclientutils/txn_recovery.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,11 @@ const (
2828
// ExpectPusheeTxnRecordNotFound means we're expecting the push to not find the
2929
// pushee txn record.
3030
ExpectPusheeTxnRecordNotFound
31+
// ExpectNoTxnRecovery means we're expecting the push to NOT perform
32+
// transaction recovery. This is weaker than ExpectPusheeTxnRecordNotFound: it
33+
// tolerates a PENDING txn record (e.g. written by the heartbeat loop) but
34+
// verifies no STAGING record was found and recovered.
35+
ExpectNoTxnRecovery
3136
// DontExpectAnything means we're not going to check the state in which the
3237
// pusher found the pushee's txn record.
3338
DontExpectAnything
@@ -114,6 +119,13 @@ func CheckPushResult(
114119
"push didn't run as expected (missing \"%s\"). recording: %s",
115120
expMsg, recording)
116121
}
122+
case ExpectNoTxnRecovery:
123+
unexpectedMsg := fmt.Sprintf("recovered txn %s", txn.ID.Short())
124+
if _, ok := recording.FindLogMessage(unexpectedMsg); ok {
125+
resolutionErr = errors.Errorf(
126+
"push unexpectedly performed txn recovery (found \"%s\"). recording: %s",
127+
unexpectedMsg, recording)
128+
}
117129
case DontExpectAnything:
118130
}
119131

0 commit comments

Comments
 (0)