From 7147c1fa5abab773450a19602f626b154357d33d Mon Sep 17 00:00:00 2001 From: Mira Radeva Date: Tue, 24 Oct 2023 14:48:37 -0400 Subject: [PATCH] backupccl: deflake TestCleanupIntentsDuringBackupPerformanceRegression The test was counting batches of pushes for the entire duration of the test, not just the back itself. This patch resets the counters to do a more targeted assertion. Fixes: #112812 Release note: None --- pkg/ccl/backupccl/backup_intents_test.go | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/pkg/ccl/backupccl/backup_intents_test.go b/pkg/ccl/backupccl/backup_intents_test.go index 610ae458f2ec..af12923d070c 100644 --- a/pkg/ccl/backupccl/backup_intents_test.go +++ b/pkg/ccl/backupccl/backup_intents_test.go @@ -100,15 +100,14 @@ func TestCleanupIntentsDuringBackupPerformanceRegression(t *testing.T) { } } + // Reset the counters to avoid counting pushes and intent resolutions not + // part of the backup. + numIntentResolveBatches.Store(0) + numPushBatches.Store(0) + _, err = sqlDb.Exec("backup table foo to 'userfile:///test.foo'") require.NoError(t, err, "Failed to run backup") - if !abort { - for _, tx := range transactions { - require.NoError(t, tx.Commit()) - } - } - // We expect each group of 10 intents to take 2 intent resolution batches: // - One intent gets discovered and added to the lock table, which forces the // abandoned txn to be pushed and added to the txnStatusCache. @@ -120,5 +119,12 @@ func TestCleanupIntentsDuringBackupPerformanceRegression(t *testing.T) { // Each of the 1,000 transactions is expected to get pushed once, but in an // actual run of the test we might see more pushes (e.g. of other transactions). require.GreaterOrEqual(t, 1100, int(numPushBatches.Load())) + + if !abort { + for _, tx := range transactions { + // Ensure the long-running transactions can commit. + require.NoError(t, tx.Commit()) + } + } }) }