Skip to content

Commit b8008aa

Browse files
committed
sql: refactor wait for version utils
The parameter ordering was inconsistent. Epic: none Release note: None
1 parent 5fcadbd commit b8008aa

File tree

4 files changed

+15
-14
lines changed

4 files changed

+15
-14
lines changed

pkg/sql/catalog/lease/helpers_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ func (m *Manager) PublishMultiple(
151151
// of the descriptors.
152152
expectedVersions := make(map[descpb.ID]descpb.DescriptorVersion)
153153
for _, id := range ids {
154-
expected, err := m.WaitForOneVersion(ctx, id, nil, base.DefaultRetryOptions())
154+
expected, err := m.WaitForOneVersion(ctx, id, nil /* regions */, base.DefaultRetryOptions())
155155
if err != nil {
156156
return nil, err
157157
}

pkg/sql/catalog/lease/lease.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ var WaitForInitialVersion = settings.RegisterBoolSetting(settings.ApplicationLev
107107
func (m *Manager) WaitForNoVersion(
108108
ctx context.Context,
109109
id descpb.ID,
110-
cachedDatabaseRegions regionliveness.CachedDatabaseRegions,
110+
regions regionliveness.CachedDatabaseRegions,
111111
retryOpts retry.Options,
112112
) error {
113113
versions := []IDVersion{
@@ -125,7 +125,7 @@ func (m *Manager) WaitForNoVersion(
125125
defer wsTracker.end()
126126
for lastCount, r := 0, retry.Start(retryOpts); r.Next(); {
127127
now := m.storage.clock.Now()
128-
detail, err := countLeasesWithDetail(ctx, m.storage.db, m.Codec(), cachedDatabaseRegions, m.settings, versions, now, true /*forAnyVersion*/)
128+
detail, err := countLeasesWithDetail(ctx, m.storage.db, m.Codec(), regions, m.settings, versions, now, true /*forAnyVersion*/)
129129
if err != nil {
130130
return err
131131
}
@@ -313,8 +313,8 @@ func countSessionsHoldingStaleDescriptor(
313313
func (m *Manager) WaitForInitialVersion(
314314
ctx context.Context,
315315
descriptorsIds descpb.IDs,
316-
retryOpts retry.Options,
317316
regions regionliveness.CachedDatabaseRegions,
317+
retryOpts retry.Options,
318318
) error {
319319
if !WaitForInitialVersion.Get(&m.settings.SV) ||
320320
!m.storage.settings.Version.IsActive(ctx, clusterversion.V25_1) {
@@ -557,7 +557,7 @@ func (m *Manager) WaitForOneVersion(
557557
// The MaxRetries and MaxDuration in retryOpts should not be set.
558558
func (m *Manager) WaitForNewVersion(
559559
ctx context.Context,
560-
descriptorId descpb.ID,
560+
id descpb.ID,
561561
regions regionliveness.CachedDatabaseRegions,
562562
retryOpts retry.Options,
563563
) (catalog.Descriptor, error) {
@@ -577,7 +577,7 @@ func (m *Manager) WaitForNewVersion(
577577
// enjoyers).
578578
for r := retry.Start(retryOpts); r.Next(); {
579579
var err error
580-
desc, err = m.maybeGetDescriptorWithoutValidation(ctx, descriptorId, true)
580+
desc, err = m.maybeGetDescriptorWithoutValidation(ctx, id, true)
581581
if err != nil {
582582
return nil, err
583583
}

pkg/sql/catalog/lease/lease_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2333,7 +2333,7 @@ func TestLeaseWithOfflineTables(t *testing.T) {
23332333
// descriptor txn will not wait for the initial version.
23342334
if expected == descpb.DescriptorState_DROP && next == descpb.DescriptorState_PUBLIC {
23352335
require.NoError(t,
2336-
execCfg.LeaseManager.WaitForInitialVersion(ctx, descpb.IDs{testTableID()}, retry.Options{}, nil))
2336+
execCfg.LeaseManager.WaitForInitialVersion(ctx, descpb.IDs{testTableID()}, nil /* regions */, retry.Options{}))
23372337
}
23382338
// Wait for the lease manager's refresh worker to have processed the
23392339
// descriptor update.

pkg/sql/conn_executor_jobs.go

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -86,11 +86,12 @@ func (ex *connExecutor) waitForNewVersionPropagation(
8686

8787
// TODO(sql-foundations): Change this back to WaitForNewVersion once we
8888
// address the flaky behavior around privilege and role membership caching.
89-
if _, err := ex.planner.LeaseMgr().WaitForOneVersion(ex.Ctx(), idVersion.ID, cachedRegions, retry.Options{
90-
InitialBackoff: time.Millisecond,
91-
MaxBackoff: time.Second,
92-
Multiplier: 1.5,
93-
}); err != nil {
89+
if _, err := ex.planner.LeaseMgr().WaitForOneVersion(ex.Ctx(), idVersion.ID, cachedRegions,
90+
retry.Options{
91+
InitialBackoff: time.Millisecond,
92+
MaxBackoff: time.Second,
93+
Multiplier: 1.5,
94+
}); err != nil {
9495
return err
9596
}
9697
}
@@ -110,11 +111,11 @@ func (ex *connExecutor) waitForInitialVersionForNewDescriptors(
110111
descriptorIDs = append(descriptorIDs, tbl.GetID())
111112
}
112113
}
113-
return ex.planner.LeaseMgr().WaitForInitialVersion(ex.Ctx(), descriptorIDs, retry.Options{
114+
return ex.planner.LeaseMgr().WaitForInitialVersion(ex.Ctx(), descriptorIDs, cachedRegions, retry.Options{
114115
InitialBackoff: time.Millisecond,
115116
MaxBackoff: time.Second,
116117
Multiplier: 1.5,
117-
}, cachedRegions)
118+
})
118119
}
119120

120121
// descIDsInSchemaChangeJobs returns all descriptor IDs with which schema change

0 commit comments

Comments
 (0)