Skip to content

Commit

Permalink
sql: unskip TestSchemaChangeGCJob
Browse files Browse the repository at this point in the history 
Informs: #85876

A few assumptions has been changed since the test was skipped. For
example, the first user defined table descriptor ID and expected
error messages. There was actually also bugs in the error assertion
in the test for some reason, that is fixed by this PR too.

Release note: None
  • Loading branch information
@chengxiong-ruan
chengxiong-ruan committed Jul 7, 2023
1 parent 42922d0 commit ebee8de
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 22 deletions.
1 change: 0 additions & 1 deletion pkg/sql/gcjob_test/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ go_test(
"//pkg/testutils",
"//pkg/testutils/jobutils",
"//pkg/testutils/serverutils",
"//pkg/testutils/skip",
"//pkg/testutils/sqlutils",
"//pkg/util/hlc",
"//pkg/util/leaktest",
Expand Down
44 changes: 23 additions & 21 deletions pkg/sql/gcjob_test/gc_job_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ import (
"github.com/cockroachdb/cockroach/pkg/testutils"
"github.com/cockroachdb/cockroach/pkg/testutils/jobutils"
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
"github.com/cockroachdb/cockroach/pkg/testutils/skip"
"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
"github.com/cockroachdb/cockroach/pkg/util/hlc"
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
Expand All @@ -58,7 +57,6 @@ import (
// TODO(pbardea): Add more testing around the timer calculations.
func TestSchemaChangeGCJob(t *testing.T) {
defer leaktest.AfterTest(t)()
skip.WithIssue(t, 60664, "flaky test")

type DropItem int
const (
Expand All @@ -78,6 +76,7 @@ func TestSchemaChangeGCJob(t *testing.T) {
for _, ttlTime := range []TTLTime{PAST, SOON, FUTURE} {
blockGC := make(chan struct{}, 1)
params := base.TestServerArgs{}
params.ScanMaxIdleTime = time.Millisecond
params.Knobs.JobsTestingKnobs = jobs.NewTestingKnobsWithShortIntervals()
params.Knobs.GCJob = &sql.GCJobTestingKnobs{
RunBeforePerformGC: func(_ jobspb.JobID) error {
Expand All @@ -90,6 +89,11 @@ func TestSchemaChangeGCJob(t *testing.T) {
defer s.Stopper().Stop(ctx)
sqlDB := sqlutils.MakeSQLRunner(db)

sqlDB.Exec(t, `SET CLUSTER SETTING sql.gc_job.wait_for_gc.interval = '1s';`)
// Refresh protected timestamp cache immediately to make MVCC GC queue to
// process GC immediately.
sqlDB.Exec(t, `SET CLUSTER SETTING kv.protectedts.poll_interval = '1s';`)

jobRegistry := s.JobRegistry().(*jobs.Registry)

sqlDB.Exec(t, "CREATE DATABASE my_db")
Expand All @@ -100,9 +104,10 @@ func TestSchemaChangeGCJob(t *testing.T) {
sqlDB.Exec(t, "ALTER TABLE my_table CONFIGURE ZONE USING gc.ttlseconds = 1")
sqlDB.Exec(t, "ALTER TABLE my_other_table CONFIGURE ZONE USING gc.ttlseconds = 1")
}
myDBID := descpb.ID(bootstrap.TestingUserDescID(2))
myTableID := descpb.ID(bootstrap.TestingUserDescID(3))
myOtherTableID := descpb.ID(bootstrap.TestingUserDescID(4))

myDBID := descpb.ID(bootstrap.TestingUserDescID(4))
myTableID := descpb.ID(bootstrap.TestingUserDescID(6))
myOtherTableID := descpb.ID(bootstrap.TestingUserDescID(7))

var myTableDesc *tabledesc.Mutable
var myOtherTableDesc *tabledesc.Mutable
Expand Down Expand Up @@ -141,7 +146,7 @@ func TestSchemaChangeGCJob(t *testing.T) {
ParentID: myTableID,
}
myTableDesc.SetPublicNonPrimaryIndexes([]descpb.IndexDescriptor{})
expectedRunningStatus = "performing garbage collection on index 2"
expectedRunningStatus = "deleting data"
case TABLE:
details = jobspb.SchemaChangeGCDetails{
Tables: []jobspb.SchemaChangeGCDetails_DroppedID{
Expand All @@ -153,7 +158,7 @@ func TestSchemaChangeGCJob(t *testing.T) {
}
myTableDesc.State = descpb.DescriptorState_DROP
myTableDesc.DropTime = dropTime
expectedRunningStatus = fmt.Sprintf("performing garbage collection on table %d", myTableID)
expectedRunningStatus = "deleting data"
case DATABASE:
details = jobspb.SchemaChangeGCDetails{
Tables: []jobspb.SchemaChangeGCDetails_DroppedID{
Expand All @@ -172,7 +177,7 @@ func TestSchemaChangeGCJob(t *testing.T) {
myTableDesc.DropTime = dropTime
myOtherTableDesc.State = descpb.DescriptorState_DROP
myOtherTableDesc.DropTime = dropTime
expectedRunningStatus = fmt.Sprintf("performing garbage collection on tables %d, %d", myTableID, myOtherTableID)
expectedRunningStatus = "deleting data"
}

if err := kvDB.Txn(ctx, func(ctx context.Context, txn *kv.Txn) error {
Expand Down Expand Up @@ -238,24 +243,21 @@ func TestSchemaChangeGCJob(t *testing.T) {
if err := sql.TestingDescsTxn(ctx, s, func(ctx context.Context, txn isql.Txn, col *descs.Collection) error {
myImm, err := col.ByID(txn.KV()).Get().Table(ctx, myTableID)
if err != nil {
if ttlTime != FUTURE && (dropItem == TABLE || dropItem == DATABASE) {
// We dropped the table, so expect it to not be found.
require.EqualError(t, err, fmt.Sprintf(`relation "[%d]" does not exist`, myTableID))
return nil
}
return err
}
if ttlTime != FUTURE && (dropItem == TABLE || dropItem == DATABASE) {
// We dropped the table, so expect it to not be found.
require.EqualError(t, err, "descriptor not found")
return nil
}
myTableDesc = tabledesc.NewBuilder(myImm.TableDesc()).BuildExistingMutableTable()
myOtherImm, err := col.ByID(txn.KV()).Get().Table(ctx, myOtherTableID)
if err != nil {
return err
}
if ttlTime != FUTURE && dropItem == DATABASE {
// We dropped the entire database, so expect none of the tables to be found.
require.EqualError(t, err, "descriptor not found")
return nil
}
if err != nil {
if ttlTime != FUTURE && dropItem == DATABASE {
// We dropped the entire database, so expect none of the tables to be found.
require.EqualError(t, err, fmt.Sprintf(`relation "[%d]" does not exist`, myOtherTableID))
return nil
}
return err
}
myOtherTableDesc = tabledesc.NewBuilder(myOtherImm.TableDesc()).BuildExistingMutableTable()
Expand Down

0 comments on commit ebee8de

Please sign in to comment.