Merge #153070

153070: sql: refactor wait for version utils r=bghal a=bghal

The parameter ordering was inconsistent.

Epic: none

Release note: None


Co-authored-by: Brendan Gerrity <[email protected]>

Author: craig[bot]

pkg/sql/catalog/lease/helpers_test.go

@@ -151,7 +151,7 @@ func (m *Manager) PublishMultiple(
 		// of the descriptors.
 		expectedVersions := make(map[descpb.ID]descpb.DescriptorVersion)
 		for _, id := range ids {
-			expected, err := m.WaitForOneVersion(ctx, id, nil, base.DefaultRetryOptions())
+			expected, err := m.WaitForOneVersion(ctx, id, nil /* regions */, base.DefaultRetryOptions())
 			if err != nil {
 				return nil, err
 			}

pkg/sql/catalog/lease/lease.go

@@ -107,7 +107,7 @@ var WaitForInitialVersion = settings.RegisterBoolSetting(settings.ApplicationLev
 func (m *Manager) WaitForNoVersion(
 	ctx context.Context,
 	id descpb.ID,
-	cachedDatabaseRegions regionliveness.CachedDatabaseRegions,
+	regions regionliveness.CachedDatabaseRegions,
 	retryOpts retry.Options,
 ) error {
 	versions := []IDVersion{
@@ -125,7 +125,7 @@ func (m *Manager) WaitForNoVersion(
 	defer wsTracker.end()
 	for lastCount, r := 0, retry.Start(retryOpts); r.Next(); {
 		now := m.storage.clock.Now()
-		detail, err := countLeasesWithDetail(ctx, m.storage.db, m.Codec(), cachedDatabaseRegions, m.settings, versions, now, true /*forAnyVersion*/)
+		detail, err := countLeasesWithDetail(ctx, m.storage.db, m.Codec(), regions, m.settings, versions, now, true /*forAnyVersion*/)
 		if err != nil {
 			return err
 		}
@@ -317,8 +317,8 @@ func countSessionsHoldingStaleDescriptor(
 func (m *Manager) WaitForInitialVersion(
 	ctx context.Context,
 	descriptorsIds descpb.IDs,
-	retryOpts retry.Options,
 	regions regionliveness.CachedDatabaseRegions,
+	retryOpts retry.Options,
 ) error {
 	if !WaitForInitialVersion.Get(&m.settings.SV) ||
 		!m.storage.settings.Version.IsActive(ctx, clusterversion.V25_1) {
@@ -561,7 +561,7 @@ func (m *Manager) WaitForOneVersion(
 // The MaxRetries and MaxDuration in retryOpts should not be set.
 func (m *Manager) WaitForNewVersion(
 	ctx context.Context,
-	descriptorId descpb.ID,
+	id descpb.ID,
 	regions regionliveness.CachedDatabaseRegions,
 	retryOpts retry.Options,
 ) (catalog.Descriptor, error) {
@@ -581,7 +581,7 @@ func (m *Manager) WaitForNewVersion(
 	// enjoyers).
 	for r := retry.Start(retryOpts); r.Next(); {
 		var err error
-		desc, err = m.maybeGetDescriptorWithoutValidation(ctx, descriptorId, true)
+		desc, err = m.maybeGetDescriptorWithoutValidation(ctx, id, true)
 		if err != nil {
 			return nil, err
 		}

pkg/sql/catalog/lease/lease_test.go

@@ -2333,7 +2333,7 @@ func TestLeaseWithOfflineTables(t *testing.T) {
 		// descriptor txn will not wait for the initial version.
 		if expected == descpb.DescriptorState_DROP && next == descpb.DescriptorState_PUBLIC {
 			require.NoError(t,
-				execCfg.LeaseManager.WaitForInitialVersion(ctx, descpb.IDs{testTableID()}, retry.Options{}, nil))
+				execCfg.LeaseManager.WaitForInitialVersion(ctx, descpb.IDs{testTableID()}, nil /* regions */, retry.Options{}))
 		}
 		// Wait for the lease manager's refresh worker to have processed the
 		// descriptor update.

pkg/sql/conn_executor_jobs.go

@@ -86,11 +86,12 @@ func (ex *connExecutor) waitForNewVersionPropagation(
 
 		// TODO(sql-foundations): Change this back to WaitForNewVersion once we
 		// address the flaky behavior around privilege and role membership caching.
-		if _, err := ex.planner.LeaseMgr().WaitForOneVersion(ex.Ctx(), idVersion.ID, cachedRegions, retry.Options{
-			InitialBackoff: time.Millisecond,
-			MaxBackoff:     time.Second,
-			Multiplier:     1.5,
-		}); err != nil {
+		if _, err := ex.planner.LeaseMgr().WaitForOneVersion(ex.Ctx(), idVersion.ID, cachedRegions,
+			retry.Options{
+				InitialBackoff: time.Millisecond,
+				MaxBackoff:     time.Second,
+				Multiplier:     1.5,
+			}); err != nil {
 			return err
 		}
 	}
@@ -110,11 +111,11 @@ func (ex *connExecutor) waitForInitialVersionForNewDescriptors(
 			descriptorIDs = append(descriptorIDs, tbl.GetID())
 		}
 	}
-	return ex.planner.LeaseMgr().WaitForInitialVersion(ex.Ctx(), descriptorIDs, retry.Options{
+	return ex.planner.LeaseMgr().WaitForInitialVersion(ex.Ctx(), descriptorIDs, cachedRegions, retry.Options{
 		InitialBackoff: time.Millisecond,
 		MaxBackoff:     time.Second,
 		Multiplier:     1.5,
-	}, cachedRegions)
+	})
 }
 
 // descIDsInSchemaChangeJobs returns all descriptor IDs with which schema change