Skip to content

Commit

Permalink
Merge #111054
Browse files Browse the repository at this point in the history
111054: sqlccl: fix a data race in TestTenantTempTableCleanup r=yuzefovich a=knz

Fixes #111028.
Epic: CRDB-6671

Release note: None

Co-authored-by: Raphael 'kena' Poss <[email protected]>
  • Loading branch information
craig[bot] and knz committed Sep 22, 2023
2 parents c24f0ae + e0b095b commit 6a2097e
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 16 deletions.
23 changes: 13 additions & 10 deletions pkg/ccl/testccl/sqlccl/temp_table_clean_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,18 +96,21 @@ func TestTenantTempTableCleanup(t *testing.T) {
)
tenantStoppers := []*stop.Stopper{stop.NewStopper(), stop.NewStopper()}

tenantSettings := cluster.MakeTestingClusterSettings()
sql.TempObjectCleanupInterval.Override(ctx, &tenantSettings.SV, time.Second)
sql.TempObjectWaitInterval.Override(ctx, &tenantSettings.SV, time.Second*0)
// Set up sessions to expire within 5 seconds of a
// nodes death.
slinstance.DefaultTTL.Override(ctx, &tenantSettings.SV, 5*time.Second)
slinstance.DefaultHeartBeat.Override(ctx, &tenantSettings.SV, time.Second)
tenantSettings := func() *cluster.Settings {
st := cluster.MakeTestingClusterSettings()
sql.TempObjectCleanupInterval.Override(ctx, &st.SV, time.Second)
sql.TempObjectWaitInterval.Override(ctx, &st.SV, time.Second*0)
// Set up sessions to expire within 5 seconds of a
// nodes death.
slinstance.DefaultTTL.Override(ctx, &st.SV, 5*time.Second)
slinstance.DefaultHeartBeat.Override(ctx, &st.SV, time.Second)
return st
}

_, tenantPrimaryDB := serverutils.StartTenant(t, tc.Server(0),
base.TestTenantArgs{
TenantID: serverutils.TestTenantID(),
Settings: tenantSettings,
Settings: tenantSettings(),
TestingKnobs: tenantTempKnobSettings,
Stopper: tenantStoppers[0],
})
Expand All @@ -124,7 +127,7 @@ func TestTenantTempTableCleanup(t *testing.T) {
_, tenantSecondDB := serverutils.StartTenant(t, tc.Server(1),
base.TestTenantArgs{
TenantID: serverutils.TestTenantID(),
Settings: tenantSettings,
Settings: tenantSettings(),
Stopper: tenantStoppers[1],
})
tenantSecondSQL := sqlutils.MakeSQLRunner(tenantSecondDB)
Expand Down Expand Up @@ -165,7 +168,7 @@ func TestTenantTempTableCleanup(t *testing.T) {
_, tenantPrimaryDB = serverutils.StartTenant(t, tc.Server(0),
base.TestTenantArgs{
TenantID: serverutils.TestTenantID(),
Settings: tenantSettings,
Settings: tenantSettings(),
TestingKnobs: tenantTempKnobSettings,
Stopper: tenantStoppers[0]})
tenantSQL = sqlutils.MakeSQLRunner(tenantPrimaryDB)
Expand Down
20 changes: 14 additions & 6 deletions pkg/settings/values.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ type Values struct {
opaque interface{}
}

type classCheck int8
type classCheck uint32

const (
// classCheckUndefined is used when the settings.Values hasn't been
Expand All @@ -67,6 +67,14 @@ const (
classCheckVirtualCluster
)

func (ck *classCheck) get() classCheck {
return classCheck(atomic.LoadUint32((*uint32)(ck)))
}

func (ck *classCheck) set(nv classCheck) {
atomic.StoreUint32((*uint32)(ck), uint32(nv))
}

const numSlots = MaxSettings + 1

type valuesContainer struct {
Expand Down Expand Up @@ -150,20 +158,20 @@ TIP: avoid using the same cluster.Settings or settings.Value object across multi
// SpecializeForSystemInterface marks the values container as
// pertaining to the system interface.
func (sv *Values) SpecializeForSystemInterface() {
if sv.classCheck != classCheckUndefined && sv.classCheck != classCheckSystemInterface {
if ck := sv.classCheck.get(); ck != classCheckUndefined && ck != classCheckSystemInterface {
panic(errors.AssertionFailedf(alreadySpecializedError))
}
sv.classCheck = classCheckSystemInterface
sv.classCheck.set(classCheckSystemInterface)
}

// SpecializeForVirtualCluster marks this container as pertaining to
// a virtual cluster, after which use of SystemOnly values is
// disallowed.
func (sv *Values) SpecializeForVirtualCluster() {
if sv.classCheck != classCheckUndefined && sv.classCheck != classCheckVirtualCluster {
if ck := sv.classCheck.get(); ck != classCheckUndefined && ck != classCheckVirtualCluster {
panic(errors.AssertionFailedf(alreadySpecializedError))
}
sv.classCheck = classCheckVirtualCluster
sv.classCheck.set(classCheckVirtualCluster)
for slot, setting := range slotTable {
if setting != nil && setting.Class() == SystemOnly {
sv.container.forbidden[slot] = true
Expand All @@ -174,7 +182,7 @@ func (sv *Values) SpecializeForVirtualCluster() {
// SpecializedToVirtualCluster returns true if this container is for a
// virtual cluster (i.e. SpecializeToVirtualCluster() was called).
func (sv *Values) SpecializedToVirtualCluster() bool {
return sv.classCheck == classCheckVirtualCluster
return sv.classCheck.get() == classCheckVirtualCluster
}

// Opaque returns the argument passed to Init.
Expand Down

0 comments on commit 6a2097e

Please sign in to comment.