Skip to content

Commit

Permalink
schemachanger: fix bug in concurrency test framework
Browse files Browse the repository at this point in the history
The newly-rewritten TestConcurrentDeclarativeSchemaChanges test had
a concurrency bug in it, which caused it to fail in cases where the
two schema changes actually take place serially. It's possible for the
later one to start after the earlier one has already finished.

Fixes: cockroachdb#106732.

Release note: None
  • Loading branch information
Marius Posta committed Jul 17, 2023
1 parent 9783626 commit e6e00c4
Showing 1 changed file with 16 additions and 20 deletions.
36 changes: 16 additions & 20 deletions pkg/sql/schemachanger/schemachanger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,32 +140,28 @@ func TestConcurrentDeclarativeSchemaChanges(t *testing.T) {
}

backfillNotif, continueNotif := initBackfillNotification()
var wg sync.WaitGroup
wg.Add(1)
go func() {
if _, err := sqlDB.Exec(`CREATE INDEX i ON t.test (v)`); err != nil {
t.Error(err)
}
wg.Done()
}()
g := ctxgroup.WithContext(ctx)
g.Go(func() error {
_, err := sqlDB.Exec(`CREATE INDEX i ON t.test (v)`)
return err
})

// Wait until the create index schema change job has started
// before kicking off the alter primary key.
// Wait until the CREATE INDEX schema change job has started
// before kicking off the ALTER PRIMARY KEY.
<-backfillNotif
require.Zero(t, alterPrimaryKeyBlockedCounter.Load())
wg.Add(1)
go func() {
if _, err := sqlDB.Exec(`ALTER TABLE t.test ALTER PRIMARY KEY USING COLUMNS (k)`); err != nil {
t.Error(err)
}
wg.Done()
}()
g.Go(func() error {
_, err := sqlDB.Exec(`ALTER TABLE t.test ALTER PRIMARY KEY USING COLUMNS (k)`)
return err
})

// Unblock the create index job.
// Unblock the CREATE INDEX, and, by extension, ALTER PRIMARY KEY.
continueNotif <- struct{}{}
wg.Wait()

// The ALTER PRIMARY KEY schema change must have been blocked.
// Wait for both schema changes to complete.
require.NoError(t, g.Wait())

// The ALTER PRIMARY KEY schema change must have blocked.
require.NotZero(t, alterPrimaryKeyBlockedCounter.Load())

// There should be 5 k/v pairs per row:
Expand Down

0 comments on commit e6e00c4

Please sign in to comment.