Merge #35723

35723: intentresolver: properly update intent meta and status after PENDING push r=nvanbenschoten a=nvanbenschoten

This change fixes a bug that was revealed by #35165. After performing
a push on PENDING transactions, `IntentResolver.CleanupTxnIntentsOnGCAsync`
would attempt to update its slice of intents to reflect the new transaction
metadata and status. This update was performed on a copy of the intents,
so it wasn't reflected in the intent slice passed to `cleanupFinishedTxnIntents`.

I believe that the effect of this is that cleaning up a PENDING transaction
would take an extra GC round.

The change to `TestGCQueueTransactionTable` makes the test fail without
the fix.

Release note: None

Co-authored-by: Nathan VanBenschoten <[email protected]>

pkg/storage/gc_queue_test.go

@@ -683,16 +683,19 @@ func TestGCQueueTransactionTable(t *testing.T) {
 		func(filterArgs storagebase.FilterArgs) *roachpb.Error {
 			if resArgs, ok := filterArgs.Req.(*roachpb.ResolveIntentRequest); ok {
 				id := string(resArgs.IntentTxn.Key)
-				var spans []roachpb.Span
-				val, ok := resolved.Load(id)
-				if ok {
-					spans = val.([]roachpb.Span)
+				// Only count finalizing intent resolution attempts in `resolved`.
+				if resArgs.Status != roachpb.PENDING {
+					var spans []roachpb.Span
+					val, ok := resolved.Load(id)
+					if ok {
+						spans = val.([]roachpb.Span)
+					}
+					spans = append(spans, roachpb.Span{
+						Key:    resArgs.Key,
+						EndKey: resArgs.EndKey,
+					})
+					resolved.Store(id, spans)
 				}
-				spans = append(spans, roachpb.Span{
-					Key:    resArgs.Key,
-					EndKey: resArgs.EndKey,
-				})
-				resolved.Store(id, spans)
 				// We've special cased one test case. Note that the intent is still
 				// counted in `resolved`.
 				if testCases[id].failResolve {

pkg/storage/intentresolver/intent_resolver.go

@@ -708,9 +708,9 @@ func (ir *IntentResolver) CleanupTxnIntentsOnGCAsync(
 				}
 				// Get the pushed txn and update the intents slice.
 				txn = &b.RawResponse().Responses[0].GetInner().(*roachpb.PushTxnResponse).PusheeTxn
-				for _, intent := range intents {
-					intent.Txn = txn.TxnMeta
-					intent.Status = txn.Status
+				for i := range intents {
+					intents[i].Txn = txn.TxnMeta
+					intents[i].Status = txn.Status
 				}
 			}
 			var onCleanupComplete func(error)