spanconfigreconciler: use fixed timestamp when reading descriptors

Recently when we changed the default value of autocommit_before_ddl to
true, we found that the chance of hitting a retry error while running
schema changes dramatically increased. The reason was because the
backgound span reconciler would need locks for the same keys that were
being modified by the schema change job itself -- most notably, the
descriptor table and descriptor ID sequence.

This patch addresses the issue by making the spanconfig reconciler use
the checkpoint timestamp from the rangefeed as the fixed timestamp for
the transaction that reads the descriptors whose spans are being
reconciled. This allows us to re-enable the autocommit setting for
logictests that run in multitenancy.

Not only does this resolve the flakes, but it seems more correct, since
the reconciler will now read the descriptors as they were at the time of
the checkpoint, without including any possible modifications that may
have been committed in between the checkpoint and actually appearing at
the other end of the rangefeed.

Release note: None

pkg/spanconfig/spanconfigreconciler/reconciler.go

@@ -522,6 +522,17 @@ func (r *incrementalReconciler) reconcile(
 			) error {
 				var err error
 
+				// Using a fixed timestamp prevents this background job from contending
+				// with foreground schema change traffic. Schema changes modify system
+				// objects like system.descriptor, system.descriptor_id_seq, and
+				// system.span_count. The spanconfig reconciler needs to read these
+				// objects also. A fixed timestamp will avoid contention, and is also
+				// more correct since this job should read descriptors using the same
+				// timestamp at which the rangefeed was checkpointed.
+				err = txn.KV().SetFixedTimestamp(ctx, checkpoint)
+				if err != nil {
+					return err
+				}
 				// TODO(irfansharif): Instead of these filter methods for missing
 				// tables and system targets that live on the Reconciler, we could
 				// move this to the SQLTranslator instead, now that the SQLTranslator

pkg/sql/logictest/logic.go

@@ -1683,16 +1683,6 @@ func (t *logicTest) newCluster(
 		tenantID := serverutils.TestTenantID()
 		conn := t.cluster.SystemLayer(0).SQLConn(t.rootT)
 
-		// TODO(rafi): Remove this setting. We're adding it since the 3node-tenant
-		// config seem to be flaky with autcommit_before_ddl = true. Disable that
-		// setting for multitenant configs while the issue is being investigated.
-		if _, err := conn.Exec(
-			"ALTER TENANT [$1] SET CLUSTER SETTING sql.defaults.autocommit_before_ddl.enabled = false",
-			tenantID.ToUint64(),
-		); err != nil {
-			t.Fatal(err)
-		}
-
 		clusterSettings := toa.clusterSettings
 		if len(clusterSettings) > 0 {
 			// We reduce the closed timestamp duration on the host tenant so that the