diff --git a/pkg/backup/backup_test.go b/pkg/backup/backup_test.go index 93af1bc6d02c..ffa30cf3d7b3 100644 --- a/pkg/backup/backup_test.go +++ b/pkg/backup/backup_test.go @@ -10502,6 +10502,7 @@ func TestBackupDBWithViewOnAdjacentDBRange(t *testing.T) { }, }) defer cleanupFn() + srv0 := tc.StorageLayer(0) s0 := tc.ApplicationLayer(0) // Speeds up the test. @@ -10532,7 +10533,7 @@ func TestBackupDBWithViewOnAdjacentDBRange(t *testing.T) { time.Sleep(5 * time.Second) // Force GC on da.t2 to advance its GC threshold. - err := s0.ForceTableGC(context.Background(), "da", "t2", s0.Clock().Now().Add(-int64(1*time.Second), 0)) + err := srv0.ForceTableGC(context.Background(), "da", "t2", s0.Clock().Now().Add(-int64(1*time.Second), 0)) require.NoError(t, err) // This statement should succeed as we are not backing up the span for dbview, diff --git a/pkg/server/testserver.go b/pkg/server/testserver.go index 3dce2011bc15..ba1e9caaffb0 100644 --- a/pkg/server/testserver.go +++ b/pkg/server/testserver.go @@ -1297,13 +1297,6 @@ func (t *testTenant) TracerI() interface{} { return t.Tracer() } -// ForceTableGC is part of the serverutils.ApplicationLayerInterface. -func (t *testTenant) ForceTableGC( - ctx context.Context, database, table string, timestamp hlc.Timestamp, -) error { - return internalForceTableGC(ctx, t, database, table, timestamp) -} - // DefaultZoneConfig is part of the serverutils.ApplicationLayerInterface. func (t *testTenant) DefaultZoneConfig() zonepb.ZoneConfig { return *t.SystemConfigProvider().GetSystemConfig().DefaultZoneConfig @@ -2294,25 +2287,16 @@ func (ts *testServer) Tracer() *tracing.Tracer { return ts.node.storeCfg.AmbientCtx.Tracer } -// ForceTableGC is part of the serverutils.ApplicationLayerInterface. +// ForceTableGC is part of the serverutils.StorageLayerInterface. func (ts *testServer) ForceTableGC( ctx context.Context, database, table string, timestamp hlc.Timestamp, ) error { - return internalForceTableGC(ctx, ts, database, table, timestamp) -} - -func internalForceTableGC( - ctx context.Context, - app serverutils.ApplicationLayerInterface, - database, table string, - timestamp hlc.Timestamp, -) error { - tableID, err := app.QueryTableID(ctx, username.RootUserName(), database, table) + tableID, err := ts.QueryTableID(ctx, username.RootUserName(), database, table) if err != nil { return err } - tblKey := app.Codec().TablePrefix(uint32(tableID)) + tblKey := ts.Codec().TablePrefix(uint32(tableID)) gcr := kvpb.GCRequest{ RequestHeader: kvpb.RequestHeader{ Key: tblKey, @@ -2320,7 +2304,7 @@ func internalForceTableGC( }, Threshold: timestamp, } - _, pErr := kv.SendWrapped(ctx, app.DistSenderI().(kv.Sender), &gcr) + _, pErr := kv.SendWrapped(ctx, ts.DistSenderI().(kv.Sender), &gcr) return pErr.GoError() } diff --git a/pkg/sql/create_stats_test.go b/pkg/sql/create_stats_test.go index 08c20c43fe9a..5ed794f72716 100644 --- a/pkg/sql/create_stats_test.go +++ b/pkg/sql/create_stats_test.go @@ -42,7 +42,7 @@ func TestStatsWithLowTTL(t *testing.T) { var blockTableReader atomic.Bool blockCh := make(chan struct{}) - s, db, _ := serverutils.StartServer(t, base.TestServerArgs{ + srv, db, _ := serverutils.StartServer(t, base.TestServerArgs{ Knobs: base.TestingKnobs{ DistSQL: &execinfra.TestingKnobs{ // Set the batch size small to avoid having to use a large @@ -59,12 +59,11 @@ func TestStatsWithLowTTL(t *testing.T) { }, }, }, - // In external-process mode the tenant doesn't have kvpb.GCRequest - // capability (and this capability can't be granted at the time of - // writing either), so we skip the external mode only. - DefaultTestTenant: base.TestSkipForExternalProcessMode(), + // ForceTableGC is only available for the system tenant. + DefaultTestTenant: base.TestIsSpecificToStorageLayerAndNeedsASystemTenant, }) - defer s.Stopper().Stop(context.Background()) + defer srv.Stopper().Stop(context.Background()) + s := srv.ApplicationLayer() r := sqlutils.MakeSQLRunner(db) r.Exec(t, `SET CLUSTER SETTING sql.stats.automatic_collection.enabled = false;`) @@ -141,7 +140,7 @@ func TestStatsWithLowTTL(t *testing.T) { nextPK++ } // Force a table GC of values older than 2 seconds. - if err := s.ForceTableGC( + if err := srv.ForceTableGC( context.Background(), "defaultdb", "t", s.Clock().Now().Add(-int64(2*time.Second), 0), ); err != nil { goroutineErr = err diff --git a/pkg/sql/user_test.go b/pkg/sql/user_test.go index 7a675ece9efa..960a3abf4605 100644 --- a/pkg/sql/user_test.go +++ b/pkg/sql/user_test.go @@ -36,10 +36,8 @@ func TestUserLoginAfterGC(t *testing.T) { ctx := context.Background() srv, db, _ := serverutils.StartServer(t, base.TestServerArgs{ - // In external-process mode the tenant doesn't have kvpb.GCRequest - // capability (and this capability can't be granted at the time of - // writing either), so we skip the external mode only. - DefaultTestTenant: base.TestSkipForExternalProcessMode(), + // ForceTableGC is only available for the system tenant. + DefaultTestTenant: base.TestIsSpecificToStorageLayerAndNeedsASystemTenant, }) defer srv.Stopper().Stop(ctx) s := srv.ApplicationLayer() @@ -55,7 +53,7 @@ func TestUserLoginAfterGC(t *testing.T) { time.Sleep(2 * time.Second) // Force a table GC with a threshold of 500ms in the past. - err = s.ForceTableGC(ctx, "system", "role_members", s.Clock().Now().Add(-int64(500*time.Millisecond), 0)) + err = srv.ForceTableGC(ctx, "system", "role_members", s.Clock().Now().Add(-int64(500*time.Millisecond), 0)) require.NoError(t, err) // Verify that newuser can still log in. diff --git a/pkg/testutils/serverutils/api.go b/pkg/testutils/serverutils/api.go index 8dcc4b5bc140..1072b51a6cd9 100644 --- a/pkg/testutils/serverutils/api.go +++ b/pkg/testutils/serverutils/api.go @@ -463,11 +463,6 @@ type ApplicationLayerInterface interface { ctx context.Context, span roachpb.Span, ) (*serverpb.TableStatsResponse, error) - // ForceTableGC forces a KV GC round on the key range for the given table. - ForceTableGC( - ctx context.Context, database, table string, timestamp hlc.Timestamp, - ) error - // DefaultZoneConfig is a convenience function that accesses // .SystemConfigProvider().GetSystemConfig().DefaultZoneConfig. DefaultZoneConfig() zonepb.ZoneConfig @@ -732,6 +727,17 @@ type StorageLayerInterface interface { // RaftConfig retrieves a copy of the raft configuration. RaftConfig() base.RaftConfig + + // ForceTableGC forces a KV GC round on the key range for the given table. + // + // Note that we don't provide this functionality as part of the + // ApplicationLayerInterface because on secondary tenants we don't put + // split points between all tables, so the KV GC request could affect other + // system tables (since they could share the underlying range with the user + // table). + ForceTableGC( + ctx context.Context, database, table string, timestamp hlc.Timestamp, + ) error } // TestServerFactory encompasses the actual implementation of the shim