Merge #159059

159059: roachtest: new test for backup with intent resolution r=stevendanna a=miraradeva

This commit adds two new roachtests that run a backup over a table with
many intents.

- `backup/intents/pending`: The intents belong to long-running pending
  transactions.
- `backup/intents/abandoned`: The intents belong to aborted
  transactions.

For both tests, we export to roachperf the duration of the backup. The
backup also runs with an aggressive timeout to catch cases of extreme
intent resolution regressions.

The test is similar to the recently removed (in f2f7482) test
`TestCleanupIntentsDuringBackupPerformanceRegression`. The deleted test
asserted on the number of intent resolution batches, which was flaky.

Closes: [#156800](#156800)

Release note: None

Co-authored-by: Mira Radeva <[email protected]>

Author: craig[bot]

pkg/cmd/roachtest/tests/backup.go

@@ -256,6 +256,180 @@ func registerBackupNodeShutdown(r registry.Registry) {
 
 }
 
+func registerBackupIntents(r registry.Registry) {
+	r.Add(registry.TestSpec{
+		Name:                      "backup/intents/pending",
+		Owner:                     registry.OwnerKV,
+		Cluster:                   r.MakeClusterSpec(4),
+		EncryptionSupport:         registry.EncryptionMetamorphic,
+		Leases:                    registry.MetamorphicLeases,
+		CompatibleClouds:          registry.Clouds(spec.GCE, spec.Local),
+		Suites:                    registry.Suites(registry.Nightly),
+		TestSelectionOptOutSuites: registry.Suites(registry.Nightly),
+		PostProcessPerfMetrics: func(test string, histogram *roachtestutil.HistogramMetric) (roachtestutil.AggregatedPerfMetrics, error) {
+			return roachtestutil.AggregatedPerfMetrics{
+				{
+					Name:           fmt.Sprintf("%s_backup_latency", test),
+					Value:          histogram.Elapsed,
+					Unit:           "ms",
+					IsHigherBetter: false,
+				},
+			}, nil
+		},
+		Run: func(ctx context.Context, t test.Test, c cluster.Cluster) {
+			c.Start(ctx, t.L(), option.NewStartOpts(option.NoBackupSchedule), install.MakeClusterSettings())
+			exporter := roachtestutil.CreateWorkloadHistogramExporter(t, c)
+			tick, perfBuf := initBulkJobPerfArtifacts(5*time.Minute, t, exporter)
+			defer roachtestutil.CloseExporter(ctx, exporter, t, c, perfBuf, c.Node(1), "")
+
+			tick()
+			runBackup(ctx, t, c, false /* abandon */)
+			tick()
+		},
+	})
+	r.Add(registry.TestSpec{
+		Name:                      "backup/intents/abandoned",
+		Owner:                     registry.OwnerKV,
+		Cluster:                   r.MakeClusterSpec(4),
+		EncryptionSupport:         registry.EncryptionMetamorphic,
+		Leases:                    registry.MetamorphicLeases,
+		CompatibleClouds:          registry.Clouds(spec.GCE, spec.Local),
+		Suites:                    registry.Suites(registry.Nightly),
+		TestSelectionOptOutSuites: registry.Suites(registry.Nightly),
+		PostProcessPerfMetrics: func(test string, histogram *roachtestutil.HistogramMetric) (roachtestutil.AggregatedPerfMetrics, error) {
+			return roachtestutil.AggregatedPerfMetrics{
+				{
+					Name:           fmt.Sprintf("%s_backup_latency", test),
+					Value:          histogram.Elapsed,
+					Unit:           "ms",
+					IsHigherBetter: false,
+				},
+			}, nil
+		},
+		Run: func(ctx context.Context, t test.Test, c cluster.Cluster) {
+			settings := install.MakeClusterSettings()
+			settings.Env = append(settings.Env, "COCKROACH_TEST_ONLY_ASYNC_INTENT_RESOLUTION_DISABLED=true")
+			c.Start(ctx, t.L(), option.NewStartOpts(option.NoBackupSchedule), settings)
+			exporter := roachtestutil.CreateWorkloadHistogramExporter(t, c)
+			tick, perfBuf := initBulkJobPerfArtifacts(5*time.Minute, t, exporter)
+			defer roachtestutil.CloseExporter(ctx, exporter, t, c, perfBuf, c.Node(1), "")
+
+			tick()
+			runBackup(ctx, t, c, true /* abandon */)
+			tick()
+		},
+	})
+}
+
+func runBackup(ctx context.Context, t test.Test, c cluster.Cluster, abandon bool) {
+	conn := c.Conn(ctx, t.L(), 1)
+	defer conn.Close()
+
+	const totalRowCount = 1000000
+	const perTransactionRowCount = 1000
+	numTxns := totalRowCount / perTransactionRowCount
+
+	_, err := conn.Exec("CREATE TABLE foo(k INT PRIMARY KEY, v INT NOT NULL)")
+	if err != nil {
+		t.Fatalf(err.Error())
+	}
+	// Create a split to ensure that most intents live on a different range than
+	// the corresponding txn record. This will trigger async intent resolution
+	// after the txn is aborted.
+	statement := fmt.Sprintf("ALTER TABLE foo SPLIT AT VALUES (%d)", numTxns)
+	_, err = conn.Exec(statement)
+	if err != nil {
+		t.Fatalf(err.Error())
+	}
+
+	// Disable buffered writes to ensure transactions write intents.
+	_, err = conn.Exec("SET CLUSTER SETTING kv.transaction.write_buffering.enabled = false")
+	if err != nil {
+		t.Fatalf(err.Error())
+	}
+
+	// Disable randomized anchor keys since the test controls in which range the
+	// transaction records live.
+	_, err = conn.Exec("SET CLUSTER SETTING kv.transaction.randomized_anchor_key.enabled=false")
+	if err != nil {
+		t.Fatalf(err.Error())
+	}
+
+	t.Status("writing intents")
+	transactions := make([]*gosql.Tx, numTxns)
+	for i := 0; i < totalRowCount; i += perTransactionRowCount {
+		txnIndex := i / perTransactionRowCount
+		var tx *gosql.Tx
+		tx, err = conn.Begin()
+		if err != nil {
+			t.Fatalf(err.Error())
+		}
+		transactions[txnIndex] = tx
+		// Anchor these transactions on the range that contains key i < numTxns.
+		statement = fmt.Sprintf("INSERT INTO foo (k, v) VALUES (%d, %d)", txnIndex, txnIndex)
+		_, err = tx.Exec(statement)
+		if err != nil {
+			t.Fatalf(err.Error())
+		}
+		baseKey := numTxns + txnIndex*perTransactionRowCount
+		for j := 0; j < perTransactionRowCount; j += 1 {
+			// Ensure the intents are on a different range than the txn record.
+			// TODO(mira): Currently, this takes ~4-5 min. We can speed up writing
+			// these intents by using generate_series.
+			statement = fmt.Sprintf("INSERT INTO foo (k, v) VALUES (%d, %d)", baseKey+j, j)
+			_, err = tx.Exec(statement)
+			if err != nil {
+				t.Fatalf(err.Error())
+			}
+		}
+	}
+
+	// For abandoned intents, abort the transactions. We have disabled async
+	// intent resolution above, so this should result in abandoned intents.
+	if abandon {
+		t.Status("aborting the intent transactions")
+		for _, tx := range transactions {
+			if err = tx.Rollback(); err != nil {
+				t.Fatalf(err.Error())
+			}
+		}
+	}
+
+	// These cluster settings ensure that ExportRequests issued by the backup
+	// processor are not delayed for too long, especially for abandon=false, when
+	// only the final high-priority ExportRequest can push the slow pending txn.
+	_, err = conn.Exec("SET CLUSTER SETTING bulkio.backup.read_with_priority_after = '500ms'")
+	if err != nil {
+		t.Fatalf(err.Error())
+	}
+	_, err = conn.Exec("SET CLUSTER SETTING bulkio.backup.read_retry_delay = '100ms'")
+	if err != nil {
+		t.Fatalf(err.Error())
+	}
+
+	// The backup runs with a timeout configured by bulkio.backup.read_timeout,
+	// which defaults to 5min. Currently, before the virtualized intent resolution
+	// work, this test takes ~2min for the pending case and ~3min for the
+	// abandoned case. The 5min timeout should be sufficient for this test, and
+	// if exceeded, will provide a signal that the intent resolution is taking
+	// longer than expected.
+	// TODO(mira): Adjust the timeout if the test flakes, and after the VIR work.
+	t.Status("running backup")
+	_, err = conn.Exec(fmt.Sprintf("BACKUP TABLE foo INTO 'nodelocal://1/%s'", destinationName(c)))
+	if err != nil {
+		t.Fatalf(err.Error())
+	}
+
+	// Commit the pending transactions after backup.
+	if !abandon {
+		for _, tx := range transactions {
+			if err = tx.Commit(); err != nil {
+				t.Fatalf(err.Error())
+			}
+		}
+	}
+}
+
 // initBulkJobPerfArtifacts registers a histogram, creates a performance
 // artifact directory and returns a method that when invoked records a tick.
 func initBulkJobPerfArtifacts(

pkg/cmd/roachtest/tests/registry.go

@@ -22,6 +22,7 @@ func RegisterTests(r registry.Registry) {
 	registerAlterPK(r)
 	registerAsyncpg(r)
 	registerBackup(r)
+	registerBackupIntents(r)
 	registerBackupMixedVersion(r)
 	registerBackupNodeShutdown(r)
 	registerBackupRestoreRoundTrip(r)

pkg/kv/kvserver/intentresolver/intent_resolver.go

@@ -35,6 +35,13 @@ import (
 	"github.com/cockroachdb/errors"
 )
 
+// asyncIntentResolutionDisabled disables asynchronous intent resolution. It is
+// similar to DisableAsyncIntentResolution in IntentResolverTestingKnobs, but
+// for the use of roachtests.
+var asyncIntentResolutionDisabled = envutil.EnvOrDefaultBool(
+	"COCKROACH_TEST_ONLY_ASYNC_INTENT_RESOLUTION_DISABLED", false,
+)
+
 // defaultTaskLimit is the maximum number of asynchronous tasks that may be
 // started by intentResolver. When this limit is reached asynchronous tasks will
 // start to block to apply backpressure.  This is a last line of defense against
@@ -491,7 +498,7 @@ func (ir *IntentResolver) MaybePushTransactions(
 func (ir *IntentResolver) runAsyncTask(
 	origCtx context.Context, allowSyncProcessing bool, taskFn func(context.Context),
 ) error {
-	if ir.testingKnobs.DisableAsyncIntentResolution {
+	if ir.testingKnobs.DisableAsyncIntentResolution || asyncIntentResolutionDisabled {
 		return errors.New("intents not processed as async resolution is disabled")
 	}
 	ctx, hdl, err := ir.stopper.GetHandle(