Skip to content

Commit 6e91815

Browse files
committed
sql: temporarily switch back to WaitForOneVersion for table version bumps
65c953b changed the waiting logic for schema changes that were version-bump-only to wait for the new version to be published to all nodes, rather than waiting for all nodes to converge to one version. That change appears to have made tests flaky, so we temporarily go back to the old logic to unblock CI. This also skips the related flaky tests. Release note: None
1 parent 431cd74 commit 6e91815

File tree

4 files changed

+12
-5
lines changed

4 files changed

+12
-5
lines changed

pkg/sql/catalog/lease/lease.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -558,8 +558,8 @@ func (m *Manager) WaitForOneVersion(
558558
func (m *Manager) WaitForNewVersion(
559559
ctx context.Context,
560560
descriptorId descpb.ID,
561-
retryOpts retry.Options,
562561
regions regionliveness.CachedDatabaseRegions,
562+
retryOpts retry.Options,
563563
) (catalog.Descriptor, error) {
564564
if retryOpts.MaxRetries != 0 {
565565
return nil, errors.New("The MaxRetries option shouldn't be set in WaitForNewVersion")

pkg/sql/catalog/lease/lease_test.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -557,6 +557,8 @@ func TestWaitForNewVersion(testingT *testing.T) {
557557
defer leaktest.AfterTest(testingT)()
558558
defer log.Scope(testingT).Close(testingT)
559559

560+
skip.WithIssue(testingT, 152051)
561+
560562
var params base.TestClusterArgs
561563
params.ServerArgs.Knobs = base.TestingKnobs{
562564
SQLLeaseManager: &lease.ManagerTestingKnobs{
@@ -586,7 +588,7 @@ func TestWaitForNewVersion(testingT *testing.T) {
586588
timeoutCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
587589
defer cancel()
588590

589-
_, err := leaseMgr.WaitForNewVersion(timeoutCtx, descID, retry.Options{}, nil)
591+
_, err := leaseMgr.WaitForNewVersion(timeoutCtx, descID, nil, retry.Options{})
590592
require.ErrorIs(t, err, context.DeadlineExceeded)
591593
}
592594

@@ -597,7 +599,7 @@ func TestWaitForNewVersion(testingT *testing.T) {
597599
require.NoError(t, t.node(2).AcquireFreshestFromStore(ctx, descID))
598600
t.expectLeases(descID, "/1/1 /1/2 /2/1 /2/2 /2/3")
599601

600-
desc, err := leaseMgr.WaitForNewVersion(context.Background(), descID, retry.Options{}, nil)
602+
desc, err := leaseMgr.WaitForNewVersion(context.Background(), descID, nil, retry.Options{})
601603
require.NoError(t, err)
602604
require.Equal(t, desc.GetVersion(), descpb.DescriptorVersion(2))
603605
}

pkg/sql/conn_executor_jobs.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,11 +84,13 @@ func (ex *connExecutor) waitForNewVersionPropagation(
8484
continue
8585
}
8686

87-
if _, err := ex.planner.LeaseMgr().WaitForNewVersion(ex.Ctx(), idVersion.ID, retry.Options{
87+
// TODO(sql-foundations): Change this back to WaitForNewVersion once we
88+
// address the flaky behavior around privilege and role membership caching.
89+
if _, err := ex.planner.LeaseMgr().WaitForOneVersion(ex.Ctx(), idVersion.ID, cachedRegions, retry.Options{
8890
InitialBackoff: time.Millisecond,
8991
MaxBackoff: time.Second,
9092
Multiplier: 1.5,
91-
}, cachedRegions); err != nil {
93+
}); err != nil {
9294
return err
9395
}
9496
}

pkg/sql/tests/allow_user_create_during_transaction_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212

1313
"github.com/cockroachdb/cockroach/pkg/base"
1414
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
15+
"github.com/cockroachdb/cockroach/pkg/testutils/skip"
1516
"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
1617
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
1718
"github.com/cockroachdb/cockroach/pkg/util/log"
@@ -24,6 +25,8 @@ func TestAllowUserCreateDuringTransaction(t *testing.T) {
2425
defer leaktest.AfterTest(t)()
2526
defer log.Scope(t).Close(t)
2627

28+
skip.WithIssue(t, 152043)
29+
2730
ctx := context.Background()
2831
s, sqlDB, _ := serverutils.StartServer(t, base.TestServerArgs{})
2932
defer s.Stopper().Stop(ctx)

0 commit comments

Comments
 (0)