From 15c9eb74bffd3804ce3cc6d315b17fdbc358a0d5 Mon Sep 17 00:00:00 2001 From: AmoebaProtozoa <8039876+AmoebaProtozoa@users.noreply.github.com> Date: Tue, 30 Jun 2026 03:07:12 +0800 Subject: [PATCH 01/28] external workload: wire GCV2 coordination --- cmd/tidb-server/main.go | 41 ++++++++++++---- pkg/executor/BUILD.bazel | 1 + pkg/executor/set.go | 14 ++++++ pkg/extworkload/BUILD.bazel | 5 ++ pkg/extworkload/global.go | 43 ++++++++++++++++ pkg/extworkload/util.go | 10 +++- pkg/extworkload/util_test.go | 41 +++++++++++++++- pkg/session/BUILD.bazel | 5 ++ pkg/session/external_workload.go | 40 +++++++++++++++ pkg/session/external_workload_test.go | 70 +++++++++++++++++++++++++++ pkg/session/session.go | 1 + pkg/session/upgrade_run.go | 2 + pkg/store/gcworker/BUILD.bazel | 2 + pkg/store/gcworker/gc_worker.go | 59 ++++++++++++++++++++++ pkg/store/gcworker/gc_worker_test.go | 65 +++++++++++++++++++++++++ 15 files changed, 387 insertions(+), 12 deletions(-) create mode 100644 pkg/extworkload/global.go create mode 100644 pkg/session/external_workload.go create mode 100644 pkg/session/external_workload_test.go diff --git a/cmd/tidb-server/main.go b/cmd/tidb-server/main.go index e117e3e138629..6bfcd13591a8b 100644 --- a/cmd/tidb-server/main.go +++ b/cmd/tidb-server/main.go @@ -336,6 +336,7 @@ func initExternalWorkloadManager(ctx context.Context, storage kv.Storage) extwor logutil.BgLogger().Warn("failed to initialize external workload manager; TiDB will continue without external workload coordination", zap.Error(err)) return nil } + extworkload.SetGlobalManager(mgr) return mgr } @@ -343,11 +344,19 @@ func closeExternalWorkloadManager(mgr extworkload.Manager) { if mgr == nil { return } + extworkload.SetGlobalManager(nil) if err := mgr.Close(); err != nil { logutil.BgLogger().Warn("failed to close external workload manager", zap.Error(err)) } } +func initExternalWorkloadService(ctx context.Context, mgr extworkload.Manager) error { + if extworkload.IsMaster(mgr) && extworkload.UseKeyspaceLevelGC(mgr) { + return mgr.InitializeGCV2(ctx) + } + return nil +} + func main() { fset := initFlagSet() if args := fset.Args(); len(args) != 0 { @@ -477,12 +486,16 @@ func main() { keyspaceName := keyspace.GetKeyspaceNameBySettings() executor.Start() resourcemanager.InstanceResourceManager.Start() - storage, dom, err := createStoreDDLOwnerMgrAndDomain(keyspaceName) + storage, dom, externalWorkloadManager, err := createStoreDDLOwnerMgrAndDomain(keyspaceName) terror.MustNil(err) + defer func() { + closeExternalWorkloadManager(externalWorkloadManager) + }() repository.SetupRepository(dom) - if deploymode.IsStarter() { - externalWorkloadManager := initExternalWorkloadManager(context.Background(), storage) - defer closeExternalWorkloadManager(externalWorkloadManager) + if err = initExternalWorkloadService(context.Background(), externalWorkloadManager); err != nil { + logutil.BgLogger().Warn("failed to initialize external workload service; TiDB will continue without external workload coordination", zap.Error(err)) + closeExternalWorkloadManager(externalWorkloadManager) + externalWorkloadManager = nil } svr := createServer(storage, dom) if standbyController != nil { @@ -611,23 +624,29 @@ func registerStores() error { return err } -func createStoreDDLOwnerMgrAndDomain(keyspaceName string) (kv.Storage, *domain.Domain, error) { +func createStoreDDLOwnerMgrAndDomain(keyspaceName string) (kv.Storage, *domain.Domain, extworkload.Manager, error) { if config.GetGlobalConfig().Store == config.StoreTypeUniStore { kv.StandAloneTiDB = true } storage := kvstore.MustInitStorage(keyspaceName) + var externalWorkloadManager extworkload.Manager + if deploymode.IsStarter() { + externalWorkloadManager = initExternalWorkloadManager(context.Background(), storage) + } if tikvStore, ok := storage.(kv.StorageWithPD); ok { pdhttpCli := tikvStore.GetPDHTTPClient() // unistore also implements kv.StorageWithPD, but it does not have PD client. if pdhttpCli != nil { pdStatus, err := pdhttpCli.GetStatus(context.Background()) if err != nil { - return nil, nil, err + closeExternalWorkloadManager(externalWorkloadManager) + return nil, nil, nil, err } if !kerneltype.IsMatch(pdStatus.KernelType) { log.Error("kernel type mismatch", zap.String("pd", pdStatus.KernelType), zap.String("tidb", kerneltype.Name())) - return nil, nil, errors.New("kernel type mismatch") + closeExternalWorkloadManager(externalWorkloadManager) + return nil, nil, nil, errors.New("kernel type mismatch") } } } @@ -636,13 +655,15 @@ func createStoreDDLOwnerMgrAndDomain(keyspaceName string) (kv.Storage, *domain.D // Bootstrap a session to load information schema. err := ddl.StartOwnerManager(context.Background(), storage) if err != nil { - return nil, nil, err + closeExternalWorkloadManager(externalWorkloadManager) + return nil, nil, nil, err } dom, err := session.BootstrapSession(storage) if err != nil { - return nil, nil, err + closeExternalWorkloadManager(externalWorkloadManager) + return nil, nil, nil, err } - return storage, dom, nil + return storage, dom, externalWorkloadManager, nil } // Prometheus push. diff --git a/pkg/executor/BUILD.bazel b/pkg/executor/BUILD.bazel index 9b26968e75bc8..0cf34e1a0cf73 100644 --- a/pkg/executor/BUILD.bazel +++ b/pkg/executor/BUILD.bazel @@ -147,6 +147,7 @@ go_library( "//pkg/expression/exprctx", "//pkg/expression/sessionexpr", "//pkg/extension", + "//pkg/extworkload", "//pkg/infoschema", "//pkg/infoschema/context", "//pkg/keyspace", diff --git a/pkg/executor/set.go b/pkg/executor/set.go index 2ea3d5e1e1ea5..d4e572b800c0e 100644 --- a/pkg/executor/set.go +++ b/pkg/executor/set.go @@ -17,12 +17,14 @@ package executor import ( "context" "strings" + "time" "github.com/pingcap/errors" "github.com/pingcap/tidb/pkg/domain" "github.com/pingcap/tidb/pkg/dxf/framework/storage" "github.com/pingcap/tidb/pkg/executor/internal/exec" "github.com/pingcap/tidb/pkg/expression" + "github.com/pingcap/tidb/pkg/extworkload" "github.com/pingcap/tidb/pkg/parser/ast" "github.com/pingcap/tidb/pkg/parser/charset" "github.com/pingcap/tidb/pkg/parser/mysql" @@ -189,6 +191,18 @@ func (e *SetExecutor) setSysVariable(ctx context.Context, name string, v *expres logstr = "set instance var" } logutil.BgLogger().Info(logstr, zap.Uint64("conn", sessionVars.ConnectionID), zap.String("name", name), zap.String("val", showValStr)) + if v.IsGlobal && name == vardef.TiDBGCLifetime { + mgr := extworkload.GetGlobalManager() + if extworkload.IsMaster(mgr) && extworkload.UseKeyspaceLevelGC(mgr) { + gcLifeTime, err := time.ParseDuration(valStr) + if err != nil { + return err + } + if err = mgr.UpdateGCLifeTime(ctx, int64(gcLifeTime/time.Second)); err != nil { + return err + } + } + } if name == vardef.TiDBServiceScope { dom := domain.GetDomain(e.Ctx()) // SetInstanceSysVar has already updated vardef.ServiceScope in the sysvar hook. diff --git a/pkg/extworkload/BUILD.bazel b/pkg/extworkload/BUILD.bazel index d50f2725e7d4b..d59cd53b72996 100644 --- a/pkg/extworkload/BUILD.bazel +++ b/pkg/extworkload/BUILD.bazel @@ -4,6 +4,7 @@ go_library( name = "extworkload", srcs = [ "external_workload.go", + "global.go", "manager.go", "util.go", ], @@ -11,11 +12,13 @@ go_library( visibility = ["//visibility:public"], deps = [ "//pkg/config", + "//pkg/config/deploymode", "//pkg/extworkload/client", "//pkg/metrics", "//pkg/util/logutil", "@com_github_pingcap_errors//:errors", "@com_github_pingcap_kvproto//pkg/keyspacepb", + "@com_github_tikv_pd_client//:client", "@org_golang_google_grpc//:grpc", "@org_uber_go_zap//:zap", ], @@ -33,6 +36,8 @@ go_test( shard_count = 6, deps = [ "//pkg/config", + "//pkg/config/deploymode", + "//pkg/config/kerneltype", "//pkg/extworkload/client", "//pkg/metrics", "@com_github_pingcap_kvproto//pkg/externalworkloadpb", diff --git a/pkg/extworkload/global.go b/pkg/extworkload/global.go new file mode 100644 index 0000000000000..ee602fb9db40f --- /dev/null +++ b/pkg/extworkload/global.go @@ -0,0 +1,43 @@ +// Copyright 2026 PingCAP, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package extworkload + +import ( + "sync" + + "github.com/pingcap/tidb/pkg/config/deploymode" +) + +var globalManager struct { + sync.RWMutex + mgr Manager +} + +// SetGlobalManager installs the process-wide external workload manager. +func SetGlobalManager(mgr Manager) { + globalManager.Lock() + globalManager.mgr = mgr + globalManager.Unlock() +} + +// GetGlobalManager returns the installed manager only in Starter deploy mode. +func GetGlobalManager() Manager { + if !deploymode.IsStarter() { + return nil + } + globalManager.RLock() + defer globalManager.RUnlock() + return globalManager.mgr +} diff --git a/pkg/extworkload/util.go b/pkg/extworkload/util.go index db17bb6a4a82f..f80fb617639c2 100644 --- a/pkg/extworkload/util.go +++ b/pkg/extworkload/util.go @@ -14,7 +14,10 @@ package extworkload -import "github.com/pingcap/tidb/pkg/config" +import ( + "github.com/pingcap/tidb/pkg/config" + pd "github.com/tikv/pd/client" +) // IsEnabled reports whether a Manager is present. func IsEnabled(m Manager) bool { return m != nil } @@ -31,6 +34,11 @@ func IsTTLTaskWorker(m Manager) bool { return roleIs(m, config.RoleTTLTaskWorker // IsAutoAnalyzeWorker reports whether this TiDB should run auto-analyze jobs. func IsAutoAnalyzeWorker(m Manager) bool { return roleIs(m, config.RoleAutoAnalyzeWorker) } +// UseKeyspaceLevelGC reports whether the manager is bound to a keyspace-level GC keyspace. +func UseKeyspaceLevelGC(m Manager) bool { + return IsEnabled(m) && m.Meta() != nil && pd.IsKeyspaceUsingKeyspaceLevelGC(m.Meta()) +} + func roleIs(m Manager, role config.ExternalWorkloadRole) bool { return IsEnabled(m) && m.Role() == role } diff --git a/pkg/extworkload/util_test.go b/pkg/extworkload/util_test.go index 613a3e3d8315c..26233ffafcba7 100644 --- a/pkg/extworkload/util_test.go +++ b/pkg/extworkload/util_test.go @@ -19,16 +19,20 @@ import ( "github.com/pingcap/kvproto/pkg/keyspacepb" "github.com/pingcap/tidb/pkg/config" + "github.com/pingcap/tidb/pkg/config/deploymode" + "github.com/pingcap/tidb/pkg/config/kerneltype" "github.com/stretchr/testify/require" + pd "github.com/tikv/pd/client" ) type stubManager struct { Manager role config.ExternalWorkloadRole + meta *keyspacepb.KeyspaceMeta } func (s *stubManager) Role() config.ExternalWorkloadRole { return s.role } -func (s *stubManager) Meta() *keyspacepb.KeyspaceMeta { return nil } +func (s *stubManager) Meta() *keyspacepb.KeyspaceMeta { return s.meta } func (s *stubManager) Close() error { return nil } func TestRolePredicatesWhenDisabled(t *testing.T) { @@ -59,3 +63,38 @@ func TestRolePredicatesDedicated(t *testing.T) { }) } } + +func TestUseKeyspaceLevelGC(t *testing.T) { + mgr := &stubManager{ + role: config.RoleMaster, + } + require.False(t, UseKeyspaceLevelGC(mgr)) + + mgr.meta = &keyspacepb.KeyspaceMeta{ + Id: 1, + Name: "ks", + Config: map[string]string{pd.KeyspaceConfigGCManagementType: pd.KeyspaceConfigGCManagementTypeKeyspaceLevel}, + } + require.True(t, UseKeyspaceLevelGC(mgr)) +} + +func TestGlobalManagerStarterGate(t *testing.T) { + if kerneltype.IsClassic() { + t.Skip("Starter deploy mode is only available in nextgen kernel") + } + + origin := deploymode.Get() + t.Cleanup(func() { + SetGlobalManager(nil) + require.NoError(t, deploymode.Set(origin)) + }) + + mgr := &stubManager{role: config.RoleMaster} + SetGlobalManager(mgr) + + require.NoError(t, deploymode.Set(deploymode.Premium)) + require.Nil(t, GetGlobalManager()) + + require.NoError(t, deploymode.Set(deploymode.Starter)) + require.Same(t, mgr, GetGlobalManager()) +} diff --git a/pkg/session/BUILD.bazel b/pkg/session/BUILD.bazel index 278f832530b28..2cf454ea3ed84 100644 --- a/pkg/session/BUILD.bazel +++ b/pkg/session/BUILD.bazel @@ -6,6 +6,7 @@ go_library( "advisory_locks.go", "bootstrap.go", "contextimpl.go", + "external_workload.go", "mock_bootstrap.go", "nontransactional.go", "session.go", @@ -47,6 +48,7 @@ go_library( "//pkg/expression/sessionexpr", "//pkg/extension", "//pkg/extension/extensionimpl", + "//pkg/extworkload", "//pkg/infoschema", "//pkg/infoschema/context", "//pkg/infoschema/issyncer", @@ -159,6 +161,7 @@ go_test( srcs = [ "bench_test.go", "bootstrap_test.go", + "external_workload_test.go", "main_test.go", "session_test.go", "tidb_test.go", @@ -173,12 +176,14 @@ go_test( "//pkg/autoid_service", "//pkg/bindinfo", "//pkg/config", + "//pkg/config/deploymode", "//pkg/config/kerneltype", "//pkg/ddl", "//pkg/domain", "//pkg/executor", "//pkg/expression", "//pkg/expression/sessionexpr", + "//pkg/extworkload", "//pkg/keyspace", "//pkg/kv", "//pkg/meta", diff --git a/pkg/session/external_workload.go b/pkg/session/external_workload.go new file mode 100644 index 0000000000000..1e038a0c1dc89 --- /dev/null +++ b/pkg/session/external_workload.go @@ -0,0 +1,40 @@ +// Copyright 2026 PingCAP, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package session + +import ( + "context" + "os" + + "github.com/pingcap/tidb/pkg/extworkload" + "github.com/pingcap/tidb/pkg/util/intest" + "github.com/pingcap/tidb/pkg/util/logutil" + "go.uber.org/zap" +) + +func abortGCV2() { + mgr := extworkload.GetGlobalManager() + if !extworkload.IsGCV2Worker(mgr) { + return + } + if err := mgr.AbortGCV2(context.Background()); err != nil { + logutil.BgLogger().Fatal("abort GCV2 worker failed", zap.Error(err)) + } + logutil.BgLogger().Info("GCV2 worker aborted") + if intest.InTest { + return + } + os.Exit(0) +} diff --git a/pkg/session/external_workload_test.go b/pkg/session/external_workload_test.go new file mode 100644 index 0000000000000..eddb408f66b69 --- /dev/null +++ b/pkg/session/external_workload_test.go @@ -0,0 +1,70 @@ +// Copyright 2026 PingCAP, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package session + +import ( + "context" + "testing" + + "github.com/pingcap/kvproto/pkg/keyspacepb" + "github.com/pingcap/tidb/pkg/config" + "github.com/pingcap/tidb/pkg/config/deploymode" + "github.com/pingcap/tidb/pkg/config/kerneltype" + "github.com/pingcap/tidb/pkg/extworkload" + "github.com/pingcap/tidb/pkg/util/intest" + "github.com/stretchr/testify/require" +) + +type gcv2AbortManager struct { + extworkload.Manager + role config.ExternalWorkloadRole + abortCalled bool +} + +func (m *gcv2AbortManager) Close() error { return nil } +func (m *gcv2AbortManager) Role() config.ExternalWorkloadRole { + return m.role +} +func (m *gcv2AbortManager) Meta() *keyspacepb.KeyspaceMeta { return nil } +func (m *gcv2AbortManager) AbortGCV2(context.Context) error { + m.abortCalled = true + return nil +} + +func TestAbortGCV2(t *testing.T) { + if kerneltype.IsClassic() { + t.Skip("Starter deploy mode is only available in nextgen kernel") + } + + originDeployMode := deploymode.Get() + originInTest := intest.InTest + t.Cleanup(func() { + extworkload.SetGlobalManager(nil) + intest.InTest = originInTest + require.NoError(t, deploymode.Set(originDeployMode)) + }) + require.NoError(t, deploymode.Set(deploymode.Starter)) + intest.InTest = true + + mgr := &gcv2AbortManager{role: config.RoleGCV2Worker} + extworkload.SetGlobalManager(mgr) + abortGCV2() + require.True(t, mgr.abortCalled) + + mgr.abortCalled = false + mgr.role = config.RoleMaster + abortGCV2() + require.False(t, mgr.abortCalled) +} diff --git a/pkg/session/session.go b/pkg/session/session.go index a414b90cd3941..164f2389c9423 100644 --- a/pkg/session/session.go +++ b/pkg/session/session.go @@ -4597,6 +4597,7 @@ func runInBootstrapSession(store kv.Storage, ver int64) { zap.Duration("cost", time.Since(startTime))) }() if startMode == ddl.Upgrade { + abortGCV2() // TODO at this time domain must not be created, else it will register server // info, and cause deadlock, we need to make sure this in a clear way logutil.BgLogger().Info("[upgrade] get owner lock to upgrade") diff --git a/pkg/session/upgrade_run.go b/pkg/session/upgrade_run.go index 28db74b0637fc..2779271aa82c3 100644 --- a/pkg/session/upgrade_run.go +++ b/pkg/session/upgrade_run.go @@ -44,6 +44,8 @@ func upgrade(s sessionapi.Session) { return } + abortGCV2() + printClusterState(s, ver) // when upgrade from v6.4.0 or earlier, enables metadata lock automatically, diff --git a/pkg/store/gcworker/BUILD.bazel b/pkg/store/gcworker/BUILD.bazel index 812e7efe25672..8fa1b0851acbb 100644 --- a/pkg/store/gcworker/BUILD.bazel +++ b/pkg/store/gcworker/BUILD.bazel @@ -12,6 +12,7 @@ go_library( "//pkg/ddl/placement", "//pkg/ddl/util", "//pkg/domain/infosync", + "//pkg/extworkload", "//pkg/kv", "//pkg/meta/model", "//pkg/metrics", @@ -67,6 +68,7 @@ go_test( "//pkg/ddl/util", "//pkg/domain", "//pkg/domain/infosync", + "//pkg/extworkload", "//pkg/kv", "//pkg/meta/model", "//pkg/session", diff --git a/pkg/store/gcworker/gc_worker.go b/pkg/store/gcworker/gc_worker.go index 34396ef4d0e13..3937a4eafa04e 100644 --- a/pkg/store/gcworker/gc_worker.go +++ b/pkg/store/gcworker/gc_worker.go @@ -37,6 +37,7 @@ import ( "github.com/pingcap/tidb/pkg/ddl/placement" "github.com/pingcap/tidb/pkg/ddl/util" "github.com/pingcap/tidb/pkg/domain/infosync" + "github.com/pingcap/tidb/pkg/extworkload" "github.com/pingcap/tidb/pkg/kv" "github.com/pingcap/tidb/pkg/meta/model" "github.com/pingcap/tidb/pkg/metrics" @@ -307,6 +308,14 @@ func (w *GCWorker) tick(ctx context.Context) { } } +func (w *GCWorker) currentKeyspaceUsesKeyspaceLevelGC() bool { + if w == nil || w.store == nil || w.store.GetCodec() == nil { + return false + } + meta := w.store.GetCodec().GetKeyspaceMeta() + return meta != nil && pd.IsKeyspaceUsingKeyspaceLevelGC(meta) +} + // getGCSafePoint returns the current gc safe point. func getGCSafePoint(ctx context.Context, pdClient pd.Client) (uint64, error) { // If there is try to set gc safepoint is 0, the interface will not set gc safepoint to 0, @@ -837,10 +846,60 @@ func (w *GCWorker) runGCJob(ctx context.Context, safePoint uint64, concurrency g return errors.Trace(err) } + err = w.notifyGCV2AfterGC(ctx, gcSafePoint) + if err != nil { + return errors.Trace(err) + } + metrics.GCHistogram.WithLabelValues(metrics.StageTotal).Observe(time.Since(startTime).Seconds()) return nil } +func (w *GCWorker) notifyGCV2AfterGC(ctx context.Context, safePoint uint64) error { + mgr := extworkload.GetGlobalManager() + if !extworkload.IsEnabled(mgr) || !w.currentKeyspaceUsesKeyspaceLevelGC() { + return nil + } + if extworkload.IsMaster(mgr) || extworkload.IsTTLTaskWorker(mgr) { + gcLifeTime, err := w.loadDurationWithDefault(gcLifeTimeKey, gcDefaultLifeTime) + if err != nil { + logutil.Logger(ctx).Error("failed to load GC life time for external workload", + zap.String("category", "gc worker"), + zap.Error(err)) + return errors.Trace(err) + } + return notifyGCV2AfterGCWithLifeTime(ctx, mgr, safePoint, int64(*gcLifeTime/time.Second)) + } + return notifyGCV2AfterGCWithLifeTime(ctx, mgr, safePoint, 0) +} + +func notifyGCV2AfterGCWithLifeTime(ctx context.Context, mgr extworkload.Manager, safePoint uint64, gcLifeTime int64) error { + if !extworkload.UseKeyspaceLevelGC(mgr) { + return nil + } + if extworkload.IsMaster(mgr) || extworkload.IsTTLTaskWorker(mgr) { + err := mgr.RegisterGCV2(ctx, safePoint, gcLifeTime) + if err != nil { + logutil.Logger(ctx).Error("failed to register GCV2 task", + zap.String("category", "gc worker"), + zap.Uint64("safePoint", safePoint), + zap.Error(err)) + return errors.Trace(err) + } + } + if extworkload.IsMaster(mgr) || extworkload.IsGCV2Worker(mgr) || extworkload.IsTTLTaskWorker(mgr) { + err := mgr.RecycleGCV2(ctx, safePoint) + if err != nil { + logutil.Logger(ctx).Error("failed to recycle GCV2 task", + zap.String("category", "gc worker"), + zap.Uint64("safePoint", safePoint), + zap.Error(err)) + return errors.Trace(err) + } + } + return nil +} + // deleteRanges processes all delete range records whose ts < safePoint in table `gc_delete_range` // `concurrency` specifies the concurrency to send NotifyDeleteRange. func (w *GCWorker) deleteRanges( diff --git a/pkg/store/gcworker/gc_worker_test.go b/pkg/store/gcworker/gc_worker_test.go index 72ca5d6e3004a..5f7116712174e 100644 --- a/pkg/store/gcworker/gc_worker_test.go +++ b/pkg/store/gcworker/gc_worker_test.go @@ -41,6 +41,7 @@ import ( "github.com/pingcap/tidb/pkg/ddl/util" "github.com/pingcap/tidb/pkg/domain" "github.com/pingcap/tidb/pkg/domain/infosync" + "github.com/pingcap/tidb/pkg/extworkload" "github.com/pingcap/tidb/pkg/kv" "github.com/pingcap/tidb/pkg/meta/model" "github.com/pingcap/tidb/pkg/session" @@ -149,6 +150,36 @@ type mockGCWorkerSuite struct { } } +type mockExternalWorkloadManager struct { + extworkload.Manager + role config.ExternalWorkloadRole + meta *keyspacepb.KeyspaceMeta + registerCalls int + registerSafePoint uint64 + registerLifeTime int64 + recycleCalls int + recycleSafePoint uint64 +} + +func (m *mockExternalWorkloadManager) Close() error { return nil } +func (m *mockExternalWorkloadManager) Role() config.ExternalWorkloadRole { + return m.role +} +func (m *mockExternalWorkloadManager) Meta() *keyspacepb.KeyspaceMeta { + return m.meta +} +func (m *mockExternalWorkloadManager) RegisterGCV2(_ context.Context, safePoint uint64, gcLifeTime int64) error { + m.registerCalls++ + m.registerSafePoint = safePoint + m.registerLifeTime = gcLifeTime + return nil +} +func (m *mockExternalWorkloadManager) RecycleGCV2(_ context.Context, safePoint uint64) error { + m.recycleCalls++ + m.recycleSafePoint = safePoint + return nil +} + type mockGCWorkerSuiteOptions struct { storeType mockstore.StoreType schemaLease time.Duration @@ -1178,6 +1209,40 @@ func TestUnifiedGCNeedsToWait(t *testing.T) { } } +func TestNotifyGCV2AfterGCWithLifeTime(t *testing.T) { + meta := &keyspacepb.KeyspaceMeta{ + Id: 1, + Name: "ks", + Config: map[string]string{pd.KeyspaceConfigGCManagementType: pd.KeyspaceConfigGCManagementTypeKeyspaceLevel}, + } + + mgr := &mockExternalWorkloadManager{role: config.RoleMaster, meta: meta} + require.NoError(t, notifyGCV2AfterGCWithLifeTime(context.Background(), mgr, 123, 600)) + require.Equal(t, 1, mgr.registerCalls) + require.Equal(t, uint64(123), mgr.registerSafePoint) + require.Equal(t, int64(600), mgr.registerLifeTime) + require.Equal(t, 1, mgr.recycleCalls) + require.Equal(t, uint64(123), mgr.recycleSafePoint) + + mgr = &mockExternalWorkloadManager{role: config.RoleGCV2Worker, meta: meta} + require.NoError(t, notifyGCV2AfterGCWithLifeTime(context.Background(), mgr, 456, 0)) + require.Zero(t, mgr.registerCalls) + require.Equal(t, 1, mgr.recycleCalls) + require.Equal(t, uint64(456), mgr.recycleSafePoint) + + mgr = &mockExternalWorkloadManager{ + role: config.RoleMaster, + meta: &keyspacepb.KeyspaceMeta{ + Id: 2, + Name: "unified", + Config: map[string]string{pd.KeyspaceConfigGCManagementType: pd.KeyspaceConfigGCManagementTypeUnified}, + }, + } + require.NoError(t, notifyGCV2AfterGCWithLifeTime(context.Background(), mgr, 789, 600)) + require.Zero(t, mgr.registerCalls) + require.Zero(t, mgr.recycleCalls) +} + func TestResolveLockRangeInfine(t *testing.T) { s := createGCWorkerSuite(t) From 8fa56b23e5fdfe6ed08f3432580f83c54d7939cd Mon Sep 17 00:00:00 2001 From: AmoebaProtozoa <8039876+AmoebaProtozoa@users.noreply.github.com> Date: Tue, 30 Jun 2026 03:15:48 +0800 Subject: [PATCH 02/28] tidb-server: keep store bootstrap signature unchanged --- cmd/tidb-server/main.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/cmd/tidb-server/main.go b/cmd/tidb-server/main.go index 6bfcd13591a8b..225e6d28ff55e 100644 --- a/cmd/tidb-server/main.go +++ b/cmd/tidb-server/main.go @@ -486,8 +486,9 @@ func main() { keyspaceName := keyspace.GetKeyspaceNameBySettings() executor.Start() resourcemanager.InstanceResourceManager.Start() - storage, dom, externalWorkloadManager, err := createStoreDDLOwnerMgrAndDomain(keyspaceName) + storage, dom, err := createStoreDDLOwnerMgrAndDomain(keyspaceName) terror.MustNil(err) + externalWorkloadManager := extworkload.GetGlobalManager() defer func() { closeExternalWorkloadManager(externalWorkloadManager) }() @@ -624,7 +625,7 @@ func registerStores() error { return err } -func createStoreDDLOwnerMgrAndDomain(keyspaceName string) (kv.Storage, *domain.Domain, extworkload.Manager, error) { +func createStoreDDLOwnerMgrAndDomain(keyspaceName string) (kv.Storage, *domain.Domain, error) { if config.GetGlobalConfig().Store == config.StoreTypeUniStore { kv.StandAloneTiDB = true } @@ -640,13 +641,13 @@ func createStoreDDLOwnerMgrAndDomain(keyspaceName string) (kv.Storage, *domain.D pdStatus, err := pdhttpCli.GetStatus(context.Background()) if err != nil { closeExternalWorkloadManager(externalWorkloadManager) - return nil, nil, nil, err + return nil, nil, err } if !kerneltype.IsMatch(pdStatus.KernelType) { log.Error("kernel type mismatch", zap.String("pd", pdStatus.KernelType), zap.String("tidb", kerneltype.Name())) closeExternalWorkloadManager(externalWorkloadManager) - return nil, nil, nil, errors.New("kernel type mismatch") + return nil, nil, errors.New("kernel type mismatch") } } } @@ -656,14 +657,14 @@ func createStoreDDLOwnerMgrAndDomain(keyspaceName string) (kv.Storage, *domain.D err := ddl.StartOwnerManager(context.Background(), storage) if err != nil { closeExternalWorkloadManager(externalWorkloadManager) - return nil, nil, nil, err + return nil, nil, err } dom, err := session.BootstrapSession(storage) if err != nil { closeExternalWorkloadManager(externalWorkloadManager) - return nil, nil, nil, err + return nil, nil, err } - return storage, dom, externalWorkloadManager, nil + return storage, dom, nil } // Prometheus push. From 4e5816772d47956bcf7062ea380bb4dab7e3dc34 Mon Sep 17 00:00:00 2001 From: AmoebaProtozoa <8039876+AmoebaProtozoa@users.noreply.github.com> Date: Tue, 30 Jun 2026 03:28:16 +0800 Subject: [PATCH 03/28] gcworker: simplify GCV2 notification flow --- pkg/store/gcworker/BUILD.bazel | 1 + pkg/store/gcworker/gc_worker.go | 54 +++++++++++++++------- pkg/store/gcworker/gc_worker_test.go | 69 +++++++++++++++++----------- 3 files changed, 80 insertions(+), 44 deletions(-) diff --git a/pkg/store/gcworker/BUILD.bazel b/pkg/store/gcworker/BUILD.bazel index 8fa1b0851acbb..7c610f54387c2 100644 --- a/pkg/store/gcworker/BUILD.bazel +++ b/pkg/store/gcworker/BUILD.bazel @@ -6,6 +6,7 @@ go_library( importpath = "github.com/pingcap/tidb/pkg/store/gcworker", visibility = ["//visibility:public"], deps = [ + "//pkg/config", "//pkg/config/deploymode", "//pkg/ddl", "//pkg/ddl/label", diff --git a/pkg/store/gcworker/gc_worker.go b/pkg/store/gcworker/gc_worker.go index 3937a4eafa04e..c6c3751b83a5f 100644 --- a/pkg/store/gcworker/gc_worker.go +++ b/pkg/store/gcworker/gc_worker.go @@ -31,6 +31,7 @@ import ( "github.com/pingcap/kvproto/pkg/keyspacepb" "github.com/pingcap/kvproto/pkg/kvrpcpb" "github.com/pingcap/kvproto/pkg/metapb" + "github.com/pingcap/tidb/pkg/config" "github.com/pingcap/tidb/pkg/config/deploymode" "github.com/pingcap/tidb/pkg/ddl" "github.com/pingcap/tidb/pkg/ddl/label" @@ -308,14 +309,6 @@ func (w *GCWorker) tick(ctx context.Context) { } } -func (w *GCWorker) currentKeyspaceUsesKeyspaceLevelGC() bool { - if w == nil || w.store == nil || w.store.GetCodec() == nil { - return false - } - meta := w.store.GetCodec().GetKeyspaceMeta() - return meta != nil && pd.IsKeyspaceUsingKeyspaceLevelGC(meta) -} - // getGCSafePoint returns the current gc safe point. func getGCSafePoint(ctx context.Context, pdClient pd.Client) (uint64, error) { // If there is try to set gc safepoint is 0, the interface will not set gc safepoint to 0, @@ -857,10 +850,17 @@ func (w *GCWorker) runGCJob(ctx context.Context, safePoint uint64, concurrency g func (w *GCWorker) notifyGCV2AfterGC(ctx context.Context, safePoint uint64) error { mgr := extworkload.GetGlobalManager() - if !extworkload.IsEnabled(mgr) || !w.currentKeyspaceUsesKeyspaceLevelGC() { + if !extworkload.UseKeyspaceLevelGC(mgr) { + return nil + } + + action := gcv2NotificationActionForRole(mgr.Role()) + if !action.register && !action.recycle { return nil } - if extworkload.IsMaster(mgr) || extworkload.IsTTLTaskWorker(mgr) { + + var gcLifeTimeSec int64 + if action.register { gcLifeTime, err := w.loadDurationWithDefault(gcLifeTimeKey, gcDefaultLifeTime) if err != nil { logutil.Logger(ctx).Error("failed to load GC life time for external workload", @@ -868,16 +868,36 @@ func (w *GCWorker) notifyGCV2AfterGC(ctx context.Context, safePoint uint64) erro zap.Error(err)) return errors.Trace(err) } - return notifyGCV2AfterGCWithLifeTime(ctx, mgr, safePoint, int64(*gcLifeTime/time.Second)) + gcLifeTimeSec = int64(*gcLifeTime / time.Second) } - return notifyGCV2AfterGCWithLifeTime(ctx, mgr, safePoint, 0) + + return notifyGCV2Controller(ctx, mgr, safePoint, gcLifeTimeSec, action) } -func notifyGCV2AfterGCWithLifeTime(ctx context.Context, mgr extworkload.Manager, safePoint uint64, gcLifeTime int64) error { - if !extworkload.UseKeyspaceLevelGC(mgr) { - return nil +type gcv2NotificationAction struct { + register bool + recycle bool +} + +func gcv2NotificationActionForRole(role config.ExternalWorkloadRole) gcv2NotificationAction { + switch role { + case config.RoleMaster, config.RoleTTLTaskWorker: + return gcv2NotificationAction{register: true, recycle: true} + case config.RoleGCV2Worker: + return gcv2NotificationAction{recycle: true} + default: + return gcv2NotificationAction{} } - if extworkload.IsMaster(mgr) || extworkload.IsTTLTaskWorker(mgr) { +} + +func notifyGCV2Controller( + ctx context.Context, + mgr extworkload.Manager, + safePoint uint64, + gcLifeTime int64, + action gcv2NotificationAction, +) error { + if action.register { err := mgr.RegisterGCV2(ctx, safePoint, gcLifeTime) if err != nil { logutil.Logger(ctx).Error("failed to register GCV2 task", @@ -887,7 +907,7 @@ func notifyGCV2AfterGCWithLifeTime(ctx context.Context, mgr extworkload.Manager, return errors.Trace(err) } } - if extworkload.IsMaster(mgr) || extworkload.IsGCV2Worker(mgr) || extworkload.IsTTLTaskWorker(mgr) { + if action.recycle { err := mgr.RecycleGCV2(ctx, safePoint) if err != nil { logutil.Logger(ctx).Error("failed to recycle GCV2 task", diff --git a/pkg/store/gcworker/gc_worker_test.go b/pkg/store/gcworker/gc_worker_test.go index 5f7116712174e..71927b313965d 100644 --- a/pkg/store/gcworker/gc_worker_test.go +++ b/pkg/store/gcworker/gc_worker_test.go @@ -152,8 +152,6 @@ type mockGCWorkerSuite struct { type mockExternalWorkloadManager struct { extworkload.Manager - role config.ExternalWorkloadRole - meta *keyspacepb.KeyspaceMeta registerCalls int registerSafePoint uint64 registerLifeTime int64 @@ -161,13 +159,6 @@ type mockExternalWorkloadManager struct { recycleSafePoint uint64 } -func (m *mockExternalWorkloadManager) Close() error { return nil } -func (m *mockExternalWorkloadManager) Role() config.ExternalWorkloadRole { - return m.role -} -func (m *mockExternalWorkloadManager) Meta() *keyspacepb.KeyspaceMeta { - return m.meta -} func (m *mockExternalWorkloadManager) RegisterGCV2(_ context.Context, safePoint uint64, gcLifeTime int64) error { m.registerCalls++ m.registerSafePoint = safePoint @@ -1209,36 +1200,60 @@ func TestUnifiedGCNeedsToWait(t *testing.T) { } } -func TestNotifyGCV2AfterGCWithLifeTime(t *testing.T) { - meta := &keyspacepb.KeyspaceMeta{ - Id: 1, - Name: "ks", - Config: map[string]string{pd.KeyspaceConfigGCManagementType: pd.KeyspaceConfigGCManagementTypeKeyspaceLevel}, +func TestGCV2NotificationActionForRole(t *testing.T) { + cases := []struct { + name string + role config.ExternalWorkloadRole + expected gcv2NotificationAction + }{ + { + name: "master registers and recycles", + role: config.RoleMaster, + expected: gcv2NotificationAction{register: true, recycle: true}, + }, + { + name: "ttl worker registers and recycles", + role: config.RoleTTLTaskWorker, + expected: gcv2NotificationAction{register: true, recycle: true}, + }, + { + name: "gcv2 worker recycles only", + role: config.RoleGCV2Worker, + expected: gcv2NotificationAction{recycle: true}, + }, + { + name: "auto analyze worker does not participate", + role: config.RoleAutoAnalyzeWorker, + expected: gcv2NotificationAction{}, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + require.Equal(t, tc.expected, gcv2NotificationActionForRole(tc.role)) + }) } +} + +func TestNotifyGCV2Controller(t *testing.T) { + ctx := context.Background() - mgr := &mockExternalWorkloadManager{role: config.RoleMaster, meta: meta} - require.NoError(t, notifyGCV2AfterGCWithLifeTime(context.Background(), mgr, 123, 600)) + mgr := &mockExternalWorkloadManager{} + require.NoError(t, notifyGCV2Controller(ctx, mgr, 123, 600, gcv2NotificationAction{register: true, recycle: true})) require.Equal(t, 1, mgr.registerCalls) require.Equal(t, uint64(123), mgr.registerSafePoint) require.Equal(t, int64(600), mgr.registerLifeTime) require.Equal(t, 1, mgr.recycleCalls) require.Equal(t, uint64(123), mgr.recycleSafePoint) - mgr = &mockExternalWorkloadManager{role: config.RoleGCV2Worker, meta: meta} - require.NoError(t, notifyGCV2AfterGCWithLifeTime(context.Background(), mgr, 456, 0)) + mgr = &mockExternalWorkloadManager{} + require.NoError(t, notifyGCV2Controller(ctx, mgr, 456, 0, gcv2NotificationAction{recycle: true})) require.Zero(t, mgr.registerCalls) require.Equal(t, 1, mgr.recycleCalls) require.Equal(t, uint64(456), mgr.recycleSafePoint) - mgr = &mockExternalWorkloadManager{ - role: config.RoleMaster, - meta: &keyspacepb.KeyspaceMeta{ - Id: 2, - Name: "unified", - Config: map[string]string{pd.KeyspaceConfigGCManagementType: pd.KeyspaceConfigGCManagementTypeUnified}, - }, - } - require.NoError(t, notifyGCV2AfterGCWithLifeTime(context.Background(), mgr, 789, 600)) + mgr = &mockExternalWorkloadManager{} + require.NoError(t, notifyGCV2Controller(ctx, mgr, 789, 600, gcv2NotificationAction{})) require.Zero(t, mgr.registerCalls) require.Zero(t, mgr.recycleCalls) } From b08d1c34627cc0415b27ea83ad5c5d3c78f48efe Mon Sep 17 00:00:00 2001 From: AmoebaProtozoa <8039876+AmoebaProtozoa@users.noreply.github.com> Date: Tue, 30 Jun 2026 03:42:07 +0800 Subject: [PATCH 04/28] external workload: tighten GCV2 wiring --- cmd/tidb-server/BUILD.bazel | 1 + cmd/tidb-server/main.go | 30 ++++++----- pkg/extworkload/util_test.go | 1 - pkg/session/bootstrap.go | 1 + pkg/session/external_workload_test.go | 3 -- pkg/session/upgrade_run.go | 2 - pkg/store/gcworker/BUILD.bazel | 1 - pkg/store/gcworker/gc_worker.go | 49 ++++++----------- pkg/store/gcworker/gc_worker_test.go | 75 ++++++--------------------- 9 files changed, 52 insertions(+), 111 deletions(-) diff --git a/cmd/tidb-server/BUILD.bazel b/cmd/tidb-server/BUILD.bazel index 9b5396426f607..107532ee8661d 100644 --- a/cmd/tidb-server/BUILD.bazel +++ b/cmd/tidb-server/BUILD.bazel @@ -76,6 +76,7 @@ go_library( "@com_github_tikv_client_go_v2//tikv", "@com_github_tikv_client_go_v2//tikvrpc", "@com_github_tikv_client_go_v2//txnkv/transaction", + "@com_github_tikv_pd_client//:client", "@org_uber_go_automaxprocs//maxprocs", "@org_uber_go_zap//:zap", ], diff --git a/cmd/tidb-server/main.go b/cmd/tidb-server/main.go index 225e6d28ff55e..f805bac43bf0a 100644 --- a/cmd/tidb-server/main.go +++ b/cmd/tidb-server/main.go @@ -98,6 +98,7 @@ import ( "github.com/tikv/client-go/v2/tikv" "github.com/tikv/client-go/v2/tikvrpc" "github.com/tikv/client-go/v2/txnkv/transaction" + pd "github.com/tikv/pd/client" "go.uber.org/automaxprocs/maxprocs" "go.uber.org/zap" ) @@ -331,6 +332,10 @@ func initExternalWorkloadManager(ctx context.Context, storage kv.Storage) extwor logutil.BgLogger().Warn("external workload controller enabled but keyspace meta is unavailable; TiDB will continue without external workload coordination") return nil } + if cfg.Role == config.RoleGCV2Worker && !pd.IsKeyspaceUsingKeyspaceLevelGC(meta) { + logutil.BgLogger().Warn("external workload GCV2 role requires keyspace-level GC; TiDB will continue without external workload coordination") + return nil + } mgr, err := extworkload.NewManager(ctx, meta, cfg) if err != nil { logutil.BgLogger().Warn("failed to initialize external workload manager; TiDB will continue without external workload coordination", zap.Error(err)) @@ -350,13 +355,6 @@ func closeExternalWorkloadManager(mgr extworkload.Manager) { } } -func initExternalWorkloadService(ctx context.Context, mgr extworkload.Manager) error { - if extworkload.IsMaster(mgr) && extworkload.UseKeyspaceLevelGC(mgr) { - return mgr.InitializeGCV2(ctx) - } - return nil -} - func main() { fset := initFlagSet() if args := fset.Args(); len(args) != 0 { @@ -489,14 +487,18 @@ func main() { storage, dom, err := createStoreDDLOwnerMgrAndDomain(keyspaceName) terror.MustNil(err) externalWorkloadManager := extworkload.GetGlobalManager() - defer func() { - closeExternalWorkloadManager(externalWorkloadManager) - }() + if externalWorkloadManager != nil { + defer func() { + closeExternalWorkloadManager(externalWorkloadManager) + }() + } repository.SetupRepository(dom) - if err = initExternalWorkloadService(context.Background(), externalWorkloadManager); err != nil { - logutil.BgLogger().Warn("failed to initialize external workload service; TiDB will continue without external workload coordination", zap.Error(err)) - closeExternalWorkloadManager(externalWorkloadManager) - externalWorkloadManager = nil + if extworkload.IsMaster(externalWorkloadManager) && extworkload.UseKeyspaceLevelGC(externalWorkloadManager) { + if err = externalWorkloadManager.InitializeGCV2(context.Background()); err != nil { + logutil.BgLogger().Warn("failed to initialize external workload service; TiDB will continue without external workload coordination", zap.Error(err)) + closeExternalWorkloadManager(externalWorkloadManager) + externalWorkloadManager = nil + } } svr := createServer(storage, dom) if standbyController != nil { diff --git a/pkg/extworkload/util_test.go b/pkg/extworkload/util_test.go index 26233ffafcba7..a6c9e09f049de 100644 --- a/pkg/extworkload/util_test.go +++ b/pkg/extworkload/util_test.go @@ -33,7 +33,6 @@ type stubManager struct { func (s *stubManager) Role() config.ExternalWorkloadRole { return s.role } func (s *stubManager) Meta() *keyspacepb.KeyspaceMeta { return s.meta } -func (s *stubManager) Close() error { return nil } func TestRolePredicatesWhenDisabled(t *testing.T) { require.False(t, IsEnabled(nil)) diff --git a/pkg/session/bootstrap.go b/pkg/session/bootstrap.go index a95cf2fee9693..a0f9b089d13c1 100644 --- a/pkg/session/bootstrap.go +++ b/pkg/session/bootstrap.go @@ -76,6 +76,7 @@ func bootstrap(s sessionapi.Session) { } // For rolling upgrade, we can't do upgrade only in the owner. if b { + abortGCV2() upgrade(s) logutil.BgLogger().Info("upgrade successful in bootstrap", zap.Duration("take time", time.Since(startTime))) diff --git a/pkg/session/external_workload_test.go b/pkg/session/external_workload_test.go index eddb408f66b69..1822095d9475e 100644 --- a/pkg/session/external_workload_test.go +++ b/pkg/session/external_workload_test.go @@ -18,7 +18,6 @@ import ( "context" "testing" - "github.com/pingcap/kvproto/pkg/keyspacepb" "github.com/pingcap/tidb/pkg/config" "github.com/pingcap/tidb/pkg/config/deploymode" "github.com/pingcap/tidb/pkg/config/kerneltype" @@ -33,11 +32,9 @@ type gcv2AbortManager struct { abortCalled bool } -func (m *gcv2AbortManager) Close() error { return nil } func (m *gcv2AbortManager) Role() config.ExternalWorkloadRole { return m.role } -func (m *gcv2AbortManager) Meta() *keyspacepb.KeyspaceMeta { return nil } func (m *gcv2AbortManager) AbortGCV2(context.Context) error { m.abortCalled = true return nil diff --git a/pkg/session/upgrade_run.go b/pkg/session/upgrade_run.go index 2779271aa82c3..28db74b0637fc 100644 --- a/pkg/session/upgrade_run.go +++ b/pkg/session/upgrade_run.go @@ -44,8 +44,6 @@ func upgrade(s sessionapi.Session) { return } - abortGCV2() - printClusterState(s, ver) // when upgrade from v6.4.0 or earlier, enables metadata lock automatically, diff --git a/pkg/store/gcworker/BUILD.bazel b/pkg/store/gcworker/BUILD.bazel index 7c610f54387c2..c9affc9ac40de 100644 --- a/pkg/store/gcworker/BUILD.bazel +++ b/pkg/store/gcworker/BUILD.bazel @@ -69,7 +69,6 @@ go_test( "//pkg/ddl/util", "//pkg/domain", "//pkg/domain/infosync", - "//pkg/extworkload", "//pkg/kv", "//pkg/meta/model", "//pkg/session", diff --git a/pkg/store/gcworker/gc_worker.go b/pkg/store/gcworker/gc_worker.go index c6c3751b83a5f..8754efcc0f104 100644 --- a/pkg/store/gcworker/gc_worker.go +++ b/pkg/store/gcworker/gc_worker.go @@ -854,13 +854,13 @@ func (w *GCWorker) notifyGCV2AfterGC(ctx context.Context, safePoint uint64) erro return nil } - action := gcv2NotificationActionForRole(mgr.Role()) - if !action.register && !action.recycle { + shouldRegister, shouldRecycle := gcv2NotificationActions(mgr.Role()) + if !shouldRegister && !shouldRecycle { return nil } var gcLifeTimeSec int64 - if action.register { + if shouldRegister { gcLifeTime, err := w.loadDurationWithDefault(gcLifeTimeKey, gcDefaultLifeTime) if err != nil { logutil.Logger(ctx).Error("failed to load GC life time for external workload", @@ -871,34 +871,8 @@ func (w *GCWorker) notifyGCV2AfterGC(ctx context.Context, safePoint uint64) erro gcLifeTimeSec = int64(*gcLifeTime / time.Second) } - return notifyGCV2Controller(ctx, mgr, safePoint, gcLifeTimeSec, action) -} - -type gcv2NotificationAction struct { - register bool - recycle bool -} - -func gcv2NotificationActionForRole(role config.ExternalWorkloadRole) gcv2NotificationAction { - switch role { - case config.RoleMaster, config.RoleTTLTaskWorker: - return gcv2NotificationAction{register: true, recycle: true} - case config.RoleGCV2Worker: - return gcv2NotificationAction{recycle: true} - default: - return gcv2NotificationAction{} - } -} - -func notifyGCV2Controller( - ctx context.Context, - mgr extworkload.Manager, - safePoint uint64, - gcLifeTime int64, - action gcv2NotificationAction, -) error { - if action.register { - err := mgr.RegisterGCV2(ctx, safePoint, gcLifeTime) + if shouldRegister { + err := mgr.RegisterGCV2(ctx, safePoint, gcLifeTimeSec) if err != nil { logutil.Logger(ctx).Error("failed to register GCV2 task", zap.String("category", "gc worker"), @@ -907,7 +881,7 @@ func notifyGCV2Controller( return errors.Trace(err) } } - if action.recycle { + if shouldRecycle { err := mgr.RecycleGCV2(ctx, safePoint) if err != nil { logutil.Logger(ctx).Error("failed to recycle GCV2 task", @@ -920,6 +894,17 @@ func notifyGCV2Controller( return nil } +func gcv2NotificationActions(role config.ExternalWorkloadRole) (register, recycle bool) { + switch role { + case config.RoleMaster, config.RoleTTLTaskWorker: + return true, true + case config.RoleGCV2Worker: + return false, true + default: + return false, false + } +} + // deleteRanges processes all delete range records whose ts < safePoint in table `gc_delete_range` // `concurrency` specifies the concurrency to send NotifyDeleteRange. func (w *GCWorker) deleteRanges( diff --git a/pkg/store/gcworker/gc_worker_test.go b/pkg/store/gcworker/gc_worker_test.go index 71927b313965d..cda4d854ab371 100644 --- a/pkg/store/gcworker/gc_worker_test.go +++ b/pkg/store/gcworker/gc_worker_test.go @@ -41,7 +41,6 @@ import ( "github.com/pingcap/tidb/pkg/ddl/util" "github.com/pingcap/tidb/pkg/domain" "github.com/pingcap/tidb/pkg/domain/infosync" - "github.com/pingcap/tidb/pkg/extworkload" "github.com/pingcap/tidb/pkg/kv" "github.com/pingcap/tidb/pkg/meta/model" "github.com/pingcap/tidb/pkg/session" @@ -150,27 +149,6 @@ type mockGCWorkerSuite struct { } } -type mockExternalWorkloadManager struct { - extworkload.Manager - registerCalls int - registerSafePoint uint64 - registerLifeTime int64 - recycleCalls int - recycleSafePoint uint64 -} - -func (m *mockExternalWorkloadManager) RegisterGCV2(_ context.Context, safePoint uint64, gcLifeTime int64) error { - m.registerCalls++ - m.registerSafePoint = safePoint - m.registerLifeTime = gcLifeTime - return nil -} -func (m *mockExternalWorkloadManager) RecycleGCV2(_ context.Context, safePoint uint64) error { - m.recycleCalls++ - m.recycleSafePoint = safePoint - return nil -} - type mockGCWorkerSuiteOptions struct { storeType mockstore.StoreType schemaLease time.Duration @@ -1200,64 +1178,45 @@ func TestUnifiedGCNeedsToWait(t *testing.T) { } } -func TestGCV2NotificationActionForRole(t *testing.T) { +func TestGCV2NotificationActions(t *testing.T) { cases := []struct { name string role config.ExternalWorkloadRole - expected gcv2NotificationAction + register bool + recycle bool }{ { - name: "master registers and recycles", + name: "master", role: config.RoleMaster, - expected: gcv2NotificationAction{register: true, recycle: true}, + register: true, + recycle: true, }, { - name: "ttl worker registers and recycles", + name: "ttl worker", role: config.RoleTTLTaskWorker, - expected: gcv2NotificationAction{register: true, recycle: true}, + register: true, + recycle: true, }, { - name: "gcv2 worker recycles only", - role: config.RoleGCV2Worker, - expected: gcv2NotificationAction{recycle: true}, + name: "gcv2 worker", + role: config.RoleGCV2Worker, + recycle: true, }, { - name: "auto analyze worker does not participate", - role: config.RoleAutoAnalyzeWorker, - expected: gcv2NotificationAction{}, + name: "auto analyze worker", + role: config.RoleAutoAnalyzeWorker, }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - require.Equal(t, tc.expected, gcv2NotificationActionForRole(tc.role)) + register, recycle := gcv2NotificationActions(tc.role) + require.Equal(t, tc.register, register) + require.Equal(t, tc.recycle, recycle) }) } } -func TestNotifyGCV2Controller(t *testing.T) { - ctx := context.Background() - - mgr := &mockExternalWorkloadManager{} - require.NoError(t, notifyGCV2Controller(ctx, mgr, 123, 600, gcv2NotificationAction{register: true, recycle: true})) - require.Equal(t, 1, mgr.registerCalls) - require.Equal(t, uint64(123), mgr.registerSafePoint) - require.Equal(t, int64(600), mgr.registerLifeTime) - require.Equal(t, 1, mgr.recycleCalls) - require.Equal(t, uint64(123), mgr.recycleSafePoint) - - mgr = &mockExternalWorkloadManager{} - require.NoError(t, notifyGCV2Controller(ctx, mgr, 456, 0, gcv2NotificationAction{recycle: true})) - require.Zero(t, mgr.registerCalls) - require.Equal(t, 1, mgr.recycleCalls) - require.Equal(t, uint64(456), mgr.recycleSafePoint) - - mgr = &mockExternalWorkloadManager{} - require.NoError(t, notifyGCV2Controller(ctx, mgr, 789, 600, gcv2NotificationAction{})) - require.Zero(t, mgr.registerCalls) - require.Zero(t, mgr.recycleCalls) -} - func TestResolveLockRangeInfine(t *testing.T) { s := createGCWorkerSuite(t) From caa2a95a3090f6463e76a9cbefc64fe0ac8e71ae Mon Sep 17 00:00:00 2001 From: AmoebaProtozoa <8039876+AmoebaProtozoa@users.noreply.github.com> Date: Tue, 30 Jun 2026 04:08:46 +0800 Subject: [PATCH 05/28] external workload: bind manager to storage --- cmd/tidb-server/main.go | 26 ++++++++------ pkg/executor/set.go | 2 +- pkg/extworkload/BUILD.bazel | 2 +- pkg/extworkload/global.go | 43 ---------------------- pkg/extworkload/store.go | 51 +++++++++++++++++++++++++++ pkg/extworkload/util_test.go | 24 ++++++++++--- pkg/session/bootstrap.go | 2 +- pkg/session/external_workload.go | 5 +-- pkg/session/external_workload_test.go | 17 ++++++--- pkg/session/session.go | 2 +- pkg/store/driver/BUILD.bazel | 1 + pkg/store/driver/tikv_driver.go | 19 ++++++++++ pkg/store/gcworker/gc_worker.go | 2 +- 13 files changed, 127 insertions(+), 69 deletions(-) delete mode 100644 pkg/extworkload/global.go create mode 100644 pkg/extworkload/store.go diff --git a/cmd/tidb-server/main.go b/cmd/tidb-server/main.go index f805bac43bf0a..2867ada8fad8b 100644 --- a/cmd/tidb-server/main.go +++ b/cmd/tidb-server/main.go @@ -341,15 +341,21 @@ func initExternalWorkloadManager(ctx context.Context, storage kv.Storage) extwor logutil.BgLogger().Warn("failed to initialize external workload manager; TiDB will continue without external workload coordination", zap.Error(err)) return nil } - extworkload.SetGlobalManager(mgr) + if !extworkload.SetManagerForStore(storage, mgr) { + logutil.BgLogger().Warn("external workload controller enabled but storage does not support external workload coordination; TiDB will continue without it") + if err := mgr.Close(); err != nil { + logutil.BgLogger().Warn("failed to close external workload manager", zap.Error(err)) + } + return nil + } return mgr } -func closeExternalWorkloadManager(mgr extworkload.Manager) { +func closeExternalWorkloadManager(storage kv.Storage, mgr extworkload.Manager) { if mgr == nil { return } - extworkload.SetGlobalManager(nil) + extworkload.SetManagerForStore(storage, nil) if err := mgr.Close(); err != nil { logutil.BgLogger().Warn("failed to close external workload manager", zap.Error(err)) } @@ -486,17 +492,17 @@ func main() { resourcemanager.InstanceResourceManager.Start() storage, dom, err := createStoreDDLOwnerMgrAndDomain(keyspaceName) terror.MustNil(err) - externalWorkloadManager := extworkload.GetGlobalManager() + externalWorkloadManager := extworkload.GetManagerFromStore(storage) if externalWorkloadManager != nil { defer func() { - closeExternalWorkloadManager(externalWorkloadManager) + closeExternalWorkloadManager(storage, externalWorkloadManager) }() } repository.SetupRepository(dom) if extworkload.IsMaster(externalWorkloadManager) && extworkload.UseKeyspaceLevelGC(externalWorkloadManager) { if err = externalWorkloadManager.InitializeGCV2(context.Background()); err != nil { logutil.BgLogger().Warn("failed to initialize external workload service; TiDB will continue without external workload coordination", zap.Error(err)) - closeExternalWorkloadManager(externalWorkloadManager) + closeExternalWorkloadManager(storage, externalWorkloadManager) externalWorkloadManager = nil } } @@ -642,13 +648,13 @@ func createStoreDDLOwnerMgrAndDomain(keyspaceName string) (kv.Storage, *domain.D if pdhttpCli != nil { pdStatus, err := pdhttpCli.GetStatus(context.Background()) if err != nil { - closeExternalWorkloadManager(externalWorkloadManager) + closeExternalWorkloadManager(storage, externalWorkloadManager) return nil, nil, err } if !kerneltype.IsMatch(pdStatus.KernelType) { log.Error("kernel type mismatch", zap.String("pd", pdStatus.KernelType), zap.String("tidb", kerneltype.Name())) - closeExternalWorkloadManager(externalWorkloadManager) + closeExternalWorkloadManager(storage, externalWorkloadManager) return nil, nil, errors.New("kernel type mismatch") } } @@ -658,12 +664,12 @@ func createStoreDDLOwnerMgrAndDomain(keyspaceName string) (kv.Storage, *domain.D // Bootstrap a session to load information schema. err := ddl.StartOwnerManager(context.Background(), storage) if err != nil { - closeExternalWorkloadManager(externalWorkloadManager) + closeExternalWorkloadManager(storage, externalWorkloadManager) return nil, nil, err } dom, err := session.BootstrapSession(storage) if err != nil { - closeExternalWorkloadManager(externalWorkloadManager) + closeExternalWorkloadManager(storage, externalWorkloadManager) return nil, nil, err } return storage, dom, nil diff --git a/pkg/executor/set.go b/pkg/executor/set.go index d4e572b800c0e..f68a7ee7b8c1d 100644 --- a/pkg/executor/set.go +++ b/pkg/executor/set.go @@ -192,7 +192,7 @@ func (e *SetExecutor) setSysVariable(ctx context.Context, name string, v *expres } logutil.BgLogger().Info(logstr, zap.Uint64("conn", sessionVars.ConnectionID), zap.String("name", name), zap.String("val", showValStr)) if v.IsGlobal && name == vardef.TiDBGCLifetime { - mgr := extworkload.GetGlobalManager() + mgr := extworkload.GetManagerFromStore(e.Ctx().GetStore()) if extworkload.IsMaster(mgr) && extworkload.UseKeyspaceLevelGC(mgr) { gcLifeTime, err := time.ParseDuration(valStr) if err != nil { diff --git a/pkg/extworkload/BUILD.bazel b/pkg/extworkload/BUILD.bazel index d59cd53b72996..4e51625687067 100644 --- a/pkg/extworkload/BUILD.bazel +++ b/pkg/extworkload/BUILD.bazel @@ -4,8 +4,8 @@ go_library( name = "extworkload", srcs = [ "external_workload.go", - "global.go", "manager.go", + "store.go", "util.go", ], importpath = "github.com/pingcap/tidb/pkg/extworkload", diff --git a/pkg/extworkload/global.go b/pkg/extworkload/global.go deleted file mode 100644 index ee602fb9db40f..0000000000000 --- a/pkg/extworkload/global.go +++ /dev/null @@ -1,43 +0,0 @@ -// Copyright 2026 PingCAP, Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package extworkload - -import ( - "sync" - - "github.com/pingcap/tidb/pkg/config/deploymode" -) - -var globalManager struct { - sync.RWMutex - mgr Manager -} - -// SetGlobalManager installs the process-wide external workload manager. -func SetGlobalManager(mgr Manager) { - globalManager.Lock() - globalManager.mgr = mgr - globalManager.Unlock() -} - -// GetGlobalManager returns the installed manager only in Starter deploy mode. -func GetGlobalManager() Manager { - if !deploymode.IsStarter() { - return nil - } - globalManager.RLock() - defer globalManager.RUnlock() - return globalManager.mgr -} diff --git a/pkg/extworkload/store.go b/pkg/extworkload/store.go new file mode 100644 index 0000000000000..f3d5f7a39f97e --- /dev/null +++ b/pkg/extworkload/store.go @@ -0,0 +1,51 @@ +// Copyright 2026 PingCAP, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package extworkload + +import "github.com/pingcap/tidb/pkg/config/deploymode" + +// ManagerProvider is implemented by storage instances that carry an external +// workload manager. +type ManagerProvider interface { + ExternalWorkloadManager() Manager +} + +// ManagerSetter is implemented by storage instances that can install an +// external workload manager. +type ManagerSetter interface { + SetExternalWorkloadManager(Manager) +} + +// SetManagerForStore installs the manager on a storage instance that supports it. +func SetManagerForStore(store any, mgr Manager) bool { + setter, ok := store.(ManagerSetter) + if !ok { + return false + } + setter.SetExternalWorkloadManager(mgr) + return true +} + +// GetManagerFromStore returns the manager bound to store only in Starter deploy mode. +func GetManagerFromStore(store any) Manager { + if !deploymode.IsStarter() { + return nil + } + provider, ok := store.(ManagerProvider) + if !ok { + return nil + } + return provider.ExternalWorkloadManager() +} diff --git a/pkg/extworkload/util_test.go b/pkg/extworkload/util_test.go index a6c9e09f049de..2558f592f4f8a 100644 --- a/pkg/extworkload/util_test.go +++ b/pkg/extworkload/util_test.go @@ -34,6 +34,18 @@ type stubManager struct { func (s *stubManager) Role() config.ExternalWorkloadRole { return s.role } func (s *stubManager) Meta() *keyspacepb.KeyspaceMeta { return s.meta } +type stubManagerStore struct { + mgr Manager +} + +func (s *stubManagerStore) SetExternalWorkloadManager(mgr Manager) { + s.mgr = mgr +} + +func (s *stubManagerStore) ExternalWorkloadManager() Manager { + return s.mgr +} + func TestRolePredicatesWhenDisabled(t *testing.T) { require.False(t, IsEnabled(nil)) require.False(t, IsMaster(nil)) @@ -77,23 +89,25 @@ func TestUseKeyspaceLevelGC(t *testing.T) { require.True(t, UseKeyspaceLevelGC(mgr)) } -func TestGlobalManagerStarterGate(t *testing.T) { +func TestManagerFromStoreStarterGate(t *testing.T) { if kerneltype.IsClassic() { t.Skip("Starter deploy mode is only available in nextgen kernel") } origin := deploymode.Get() t.Cleanup(func() { - SetGlobalManager(nil) require.NoError(t, deploymode.Set(origin)) }) mgr := &stubManager{role: config.RoleMaster} - SetGlobalManager(mgr) + store := &stubManagerStore{} + require.True(t, SetManagerForStore(store, mgr)) require.NoError(t, deploymode.Set(deploymode.Premium)) - require.Nil(t, GetGlobalManager()) + require.Nil(t, GetManagerFromStore(store)) require.NoError(t, deploymode.Set(deploymode.Starter)) - require.Same(t, mgr, GetGlobalManager()) + require.Same(t, mgr, GetManagerFromStore(store)) + require.True(t, SetManagerForStore(store, nil)) + require.Nil(t, GetManagerFromStore(store)) } diff --git a/pkg/session/bootstrap.go b/pkg/session/bootstrap.go index a0f9b089d13c1..f651978a791e1 100644 --- a/pkg/session/bootstrap.go +++ b/pkg/session/bootstrap.go @@ -76,7 +76,7 @@ func bootstrap(s sessionapi.Session) { } // For rolling upgrade, we can't do upgrade only in the owner. if b { - abortGCV2() + abortGCV2(s.GetStore()) upgrade(s) logutil.BgLogger().Info("upgrade successful in bootstrap", zap.Duration("take time", time.Since(startTime))) diff --git a/pkg/session/external_workload.go b/pkg/session/external_workload.go index 1e038a0c1dc89..ba216f130e27f 100644 --- a/pkg/session/external_workload.go +++ b/pkg/session/external_workload.go @@ -19,13 +19,14 @@ import ( "os" "github.com/pingcap/tidb/pkg/extworkload" + "github.com/pingcap/tidb/pkg/kv" "github.com/pingcap/tidb/pkg/util/intest" "github.com/pingcap/tidb/pkg/util/logutil" "go.uber.org/zap" ) -func abortGCV2() { - mgr := extworkload.GetGlobalManager() +func abortGCV2(store kv.Storage) { + mgr := extworkload.GetManagerFromStore(store) if !extworkload.IsGCV2Worker(mgr) { return } diff --git a/pkg/session/external_workload_test.go b/pkg/session/external_workload_test.go index 1822095d9475e..b692351c8cd1b 100644 --- a/pkg/session/external_workload_test.go +++ b/pkg/session/external_workload_test.go @@ -22,6 +22,7 @@ import ( "github.com/pingcap/tidb/pkg/config/deploymode" "github.com/pingcap/tidb/pkg/config/kerneltype" "github.com/pingcap/tidb/pkg/extworkload" + "github.com/pingcap/tidb/pkg/kv" "github.com/pingcap/tidb/pkg/util/intest" "github.com/stretchr/testify/require" ) @@ -40,6 +41,15 @@ func (m *gcv2AbortManager) AbortGCV2(context.Context) error { return nil } +type gcv2AbortStore struct { + kv.Storage + mgr extworkload.Manager +} + +func (s *gcv2AbortStore) ExternalWorkloadManager() extworkload.Manager { + return s.mgr +} + func TestAbortGCV2(t *testing.T) { if kerneltype.IsClassic() { t.Skip("Starter deploy mode is only available in nextgen kernel") @@ -48,7 +58,6 @@ func TestAbortGCV2(t *testing.T) { originDeployMode := deploymode.Get() originInTest := intest.InTest t.Cleanup(func() { - extworkload.SetGlobalManager(nil) intest.InTest = originInTest require.NoError(t, deploymode.Set(originDeployMode)) }) @@ -56,12 +65,12 @@ func TestAbortGCV2(t *testing.T) { intest.InTest = true mgr := &gcv2AbortManager{role: config.RoleGCV2Worker} - extworkload.SetGlobalManager(mgr) - abortGCV2() + store := &gcv2AbortStore{mgr: mgr} + abortGCV2(store) require.True(t, mgr.abortCalled) mgr.abortCalled = false mgr.role = config.RoleMaster - abortGCV2() + abortGCV2(store) require.False(t, mgr.abortCalled) } diff --git a/pkg/session/session.go b/pkg/session/session.go index 164f2389c9423..ed86c35b1b6d8 100644 --- a/pkg/session/session.go +++ b/pkg/session/session.go @@ -4597,7 +4597,7 @@ func runInBootstrapSession(store kv.Storage, ver int64) { zap.Duration("cost", time.Since(startTime))) }() if startMode == ddl.Upgrade { - abortGCV2() + abortGCV2(store) // TODO at this time domain must not be created, else it will register server // info, and cause deadlock, we need to make sure this in a clear way logutil.BgLogger().Info("[upgrade] get owner lock to upgrade") diff --git a/pkg/store/driver/BUILD.bazel b/pkg/store/driver/BUILD.bazel index 815d866b9d96a..af1ce3a22ca93 100644 --- a/pkg/store/driver/BUILD.bazel +++ b/pkg/store/driver/BUILD.bazel @@ -10,6 +10,7 @@ go_library( "//pkg/metaservice", "//pkg/metrics", "//pkg/metrics/common", + "//pkg/extworkload", "//pkg/sessionctx/variable", "//pkg/store/copr", "//pkg/store/driver/error", diff --git a/pkg/store/driver/tikv_driver.go b/pkg/store/driver/tikv_driver.go index 4e5d4601053c9..8365bb2307049 100644 --- a/pkg/store/driver/tikv_driver.go +++ b/pkg/store/driver/tikv_driver.go @@ -26,6 +26,7 @@ import ( "github.com/pingcap/errors" deadlockpb "github.com/pingcap/kvproto/pkg/deadlock" "github.com/pingcap/kvproto/pkg/kvrpcpb" + "github.com/pingcap/tidb/pkg/extworkload" "github.com/pingcap/tidb/pkg/kv" "github.com/pingcap/tidb/pkg/metaservice" "github.com/pingcap/tidb/pkg/metrics" @@ -320,9 +321,27 @@ type tikvStore struct { keyspace string closed bool + externalWorkload struct { + sync.RWMutex + mgr extworkload.Manager + } metaServiceInfo *metaservice.Info } +// SetExternalWorkloadManager installs the external workload manager bound to this store. +func (s *tikvStore) SetExternalWorkloadManager(mgr extworkload.Manager) { + s.externalWorkload.Lock() + s.externalWorkload.mgr = mgr + s.externalWorkload.Unlock() +} + +// ExternalWorkloadManager returns the external workload manager bound to this store. +func (s *tikvStore) ExternalWorkloadManager() extworkload.Manager { + s.externalWorkload.RLock() + defer s.externalWorkload.RUnlock() + return s.externalWorkload.mgr +} + // GetOption wraps around sync.Map. func (s *tikvStore) GetOption(k any) (any, bool) { return s.opts.Load(k) diff --git a/pkg/store/gcworker/gc_worker.go b/pkg/store/gcworker/gc_worker.go index 8754efcc0f104..374de72a60324 100644 --- a/pkg/store/gcworker/gc_worker.go +++ b/pkg/store/gcworker/gc_worker.go @@ -849,7 +849,7 @@ func (w *GCWorker) runGCJob(ctx context.Context, safePoint uint64, concurrency g } func (w *GCWorker) notifyGCV2AfterGC(ctx context.Context, safePoint uint64) error { - mgr := extworkload.GetGlobalManager() + mgr := extworkload.GetManagerFromStore(w.store) if !extworkload.UseKeyspaceLevelGC(mgr) { return nil } From 780583e2e81e4480c1a6db8e88d39a18b20a65d0 Mon Sep 17 00:00:00 2001 From: AmoebaProtozoa <8039876+AmoebaProtozoa@users.noreply.github.com> Date: Tue, 30 Jun 2026 04:23:25 +0800 Subject: [PATCH 06/28] external workload: simplify GCV2 startup wiring --- cmd/tidb-server/main.go | 15 ++---- pkg/extworkload/BUILD.bazel | 1 - pkg/extworkload/store.go | 51 ------------------ pkg/extworkload/util.go | 35 ++++++++++++ pkg/session/BUILD.bazel | 3 +- pkg/session/external_workload.go | 26 +++++++-- pkg/session/external_workload_test.go | 77 ++++++++++++++++----------- 7 files changed, 109 insertions(+), 99 deletions(-) delete mode 100644 pkg/extworkload/store.go diff --git a/cmd/tidb-server/main.go b/cmd/tidb-server/main.go index 2867ada8fad8b..135c62e533e66 100644 --- a/cmd/tidb-server/main.go +++ b/cmd/tidb-server/main.go @@ -322,6 +322,9 @@ func initDeployMode(cfg *config.Config) error { } func initExternalWorkloadManager(ctx context.Context, storage kv.Storage) extworkload.Manager { + if !deploymode.IsStarter() { + return nil + } cfg := config.GetGlobalConfig().ExternalWorkload if !cfg.Enable { return nil @@ -492,13 +495,13 @@ func main() { resourcemanager.InstanceResourceManager.Start() storage, dom, err := createStoreDDLOwnerMgrAndDomain(keyspaceName) terror.MustNil(err) - externalWorkloadManager := extworkload.GetManagerFromStore(storage) + repository.SetupRepository(dom) + externalWorkloadManager := initExternalWorkloadManager(context.Background(), storage) if externalWorkloadManager != nil { defer func() { closeExternalWorkloadManager(storage, externalWorkloadManager) }() } - repository.SetupRepository(dom) if extworkload.IsMaster(externalWorkloadManager) && extworkload.UseKeyspaceLevelGC(externalWorkloadManager) { if err = externalWorkloadManager.InitializeGCV2(context.Background()); err != nil { logutil.BgLogger().Warn("failed to initialize external workload service; TiDB will continue without external workload coordination", zap.Error(err)) @@ -638,23 +641,17 @@ func createStoreDDLOwnerMgrAndDomain(keyspaceName string) (kv.Storage, *domain.D kv.StandAloneTiDB = true } storage := kvstore.MustInitStorage(keyspaceName) - var externalWorkloadManager extworkload.Manager - if deploymode.IsStarter() { - externalWorkloadManager = initExternalWorkloadManager(context.Background(), storage) - } if tikvStore, ok := storage.(kv.StorageWithPD); ok { pdhttpCli := tikvStore.GetPDHTTPClient() // unistore also implements kv.StorageWithPD, but it does not have PD client. if pdhttpCli != nil { pdStatus, err := pdhttpCli.GetStatus(context.Background()) if err != nil { - closeExternalWorkloadManager(storage, externalWorkloadManager) return nil, nil, err } if !kerneltype.IsMatch(pdStatus.KernelType) { log.Error("kernel type mismatch", zap.String("pd", pdStatus.KernelType), zap.String("tidb", kerneltype.Name())) - closeExternalWorkloadManager(storage, externalWorkloadManager) return nil, nil, errors.New("kernel type mismatch") } } @@ -664,12 +661,10 @@ func createStoreDDLOwnerMgrAndDomain(keyspaceName string) (kv.Storage, *domain.D // Bootstrap a session to load information schema. err := ddl.StartOwnerManager(context.Background(), storage) if err != nil { - closeExternalWorkloadManager(storage, externalWorkloadManager) return nil, nil, err } dom, err := session.BootstrapSession(storage) if err != nil { - closeExternalWorkloadManager(storage, externalWorkloadManager) return nil, nil, err } return storage, dom, nil diff --git a/pkg/extworkload/BUILD.bazel b/pkg/extworkload/BUILD.bazel index 4e51625687067..733833e22140c 100644 --- a/pkg/extworkload/BUILD.bazel +++ b/pkg/extworkload/BUILD.bazel @@ -5,7 +5,6 @@ go_library( srcs = [ "external_workload.go", "manager.go", - "store.go", "util.go", ], importpath = "github.com/pingcap/tidb/pkg/extworkload", diff --git a/pkg/extworkload/store.go b/pkg/extworkload/store.go deleted file mode 100644 index f3d5f7a39f97e..0000000000000 --- a/pkg/extworkload/store.go +++ /dev/null @@ -1,51 +0,0 @@ -// Copyright 2026 PingCAP, Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package extworkload - -import "github.com/pingcap/tidb/pkg/config/deploymode" - -// ManagerProvider is implemented by storage instances that carry an external -// workload manager. -type ManagerProvider interface { - ExternalWorkloadManager() Manager -} - -// ManagerSetter is implemented by storage instances that can install an -// external workload manager. -type ManagerSetter interface { - SetExternalWorkloadManager(Manager) -} - -// SetManagerForStore installs the manager on a storage instance that supports it. -func SetManagerForStore(store any, mgr Manager) bool { - setter, ok := store.(ManagerSetter) - if !ok { - return false - } - setter.SetExternalWorkloadManager(mgr) - return true -} - -// GetManagerFromStore returns the manager bound to store only in Starter deploy mode. -func GetManagerFromStore(store any) Manager { - if !deploymode.IsStarter() { - return nil - } - provider, ok := store.(ManagerProvider) - if !ok { - return nil - } - return provider.ExternalWorkloadManager() -} diff --git a/pkg/extworkload/util.go b/pkg/extworkload/util.go index f80fb617639c2..bca51d919bdc7 100644 --- a/pkg/extworkload/util.go +++ b/pkg/extworkload/util.go @@ -16,6 +16,7 @@ package extworkload import ( "github.com/pingcap/tidb/pkg/config" + "github.com/pingcap/tidb/pkg/config/deploymode" pd "github.com/tikv/pd/client" ) @@ -39,6 +40,40 @@ func UseKeyspaceLevelGC(m Manager) bool { return IsEnabled(m) && m.Meta() != nil && pd.IsKeyspaceUsingKeyspaceLevelGC(m.Meta()) } +// ManagerProvider is implemented by storage instances that carry an external +// workload manager. +type ManagerProvider interface { + ExternalWorkloadManager() Manager +} + +// ManagerSetter is implemented by storage instances that can install an +// external workload manager. +type ManagerSetter interface { + SetExternalWorkloadManager(Manager) +} + +// SetManagerForStore installs the manager on a storage instance that supports it. +func SetManagerForStore(store any, mgr Manager) bool { + setter, ok := store.(ManagerSetter) + if !ok { + return false + } + setter.SetExternalWorkloadManager(mgr) + return true +} + +// GetManagerFromStore returns the manager bound to store only in Starter deploy mode. +func GetManagerFromStore(store any) Manager { + if !deploymode.IsStarter() { + return nil + } + provider, ok := store.(ManagerProvider) + if !ok { + return nil + } + return provider.ExternalWorkloadManager() +} + func roleIs(m Manager, role config.ExternalWorkloadRole) bool { return IsEnabled(m) && m.Role() == role } diff --git a/pkg/session/BUILD.bazel b/pkg/session/BUILD.bazel index 2cf454ea3ed84..52db78cabfa9e 100644 --- a/pkg/session/BUILD.bazel +++ b/pkg/session/BUILD.bazel @@ -148,6 +148,7 @@ go_library( "@com_github_tikv_client_go_v2//trace", "@com_github_tikv_client_go_v2//txnkv/transaction", "@com_github_tikv_client_go_v2//util", + "@com_github_tikv_pd_client//:client", "@io_etcd_go_etcd_client_v3//:client", "@org_uber_go_atomic//:atomic", "@org_uber_go_zap//:zap", @@ -183,7 +184,6 @@ go_test( "//pkg/executor", "//pkg/expression", "//pkg/expression/sessionexpr", - "//pkg/extworkload", "//pkg/keyspace", "//pkg/kv", "//pkg/meta", @@ -218,6 +218,7 @@ go_test( "@com_github_pingcap_log//:log", "@com_github_stretchr_testify//require", "@com_github_tikv_client_go_v2//tikv", + "@com_github_tikv_pd_client//:client", "@io_etcd_go_etcd_tests_v3//integration", "@org_uber_go_atomic//:atomic", #keep "@org_uber_go_goleak//:goleak", diff --git a/pkg/session/external_workload.go b/pkg/session/external_workload.go index ba216f130e27f..b9ce014955f1f 100644 --- a/pkg/session/external_workload.go +++ b/pkg/session/external_workload.go @@ -18,20 +18,38 @@ import ( "context" "os" + "github.com/pingcap/tidb/pkg/config" + "github.com/pingcap/tidb/pkg/config/deploymode" "github.com/pingcap/tidb/pkg/extworkload" "github.com/pingcap/tidb/pkg/kv" "github.com/pingcap/tidb/pkg/util/intest" "github.com/pingcap/tidb/pkg/util/logutil" + pd "github.com/tikv/pd/client" "go.uber.org/zap" ) func abortGCV2(store kv.Storage) { - mgr := extworkload.GetManagerFromStore(store) - if !extworkload.IsGCV2Worker(mgr) { + if !deploymode.IsStarter() { return } - if err := mgr.AbortGCV2(context.Background()); err != nil { - logutil.BgLogger().Fatal("abort GCV2 worker failed", zap.Error(err)) + cfg := config.GetGlobalConfig().ExternalWorkload + if !cfg.Enable || cfg.Role != config.RoleGCV2Worker { + return + } + meta := store.GetCodec().GetKeyspaceMeta() + if !pd.IsKeyspaceUsingKeyspaceLevelGC(meta) { + return + } + mgr, err := extworkload.NewManager(context.Background(), meta, cfg) + if err != nil { + logutil.BgLogger().Fatal("initialize external workload manager for GCV2 abort failed", zap.Error(err)) + } + abortErr := mgr.AbortGCV2(context.Background()) + if err := mgr.Close(); err != nil { + logutil.BgLogger().Warn("failed to close external workload manager after GCV2 abort", zap.Error(err)) + } + if abortErr != nil { + logutil.BgLogger().Fatal("abort GCV2 worker failed", zap.Error(abortErr)) } logutil.BgLogger().Info("GCV2 worker aborted") if intest.InTest { diff --git a/pkg/session/external_workload_test.go b/pkg/session/external_workload_test.go index b692351c8cd1b..e984e1c3376c5 100644 --- a/pkg/session/external_workload_test.go +++ b/pkg/session/external_workload_test.go @@ -15,62 +15,75 @@ package session import ( - "context" "testing" + "github.com/pingcap/kvproto/pkg/keyspacepb" "github.com/pingcap/tidb/pkg/config" "github.com/pingcap/tidb/pkg/config/deploymode" "github.com/pingcap/tidb/pkg/config/kerneltype" - "github.com/pingcap/tidb/pkg/extworkload" "github.com/pingcap/tidb/pkg/kv" - "github.com/pingcap/tidb/pkg/util/intest" "github.com/stretchr/testify/require" + "github.com/tikv/client-go/v2/tikv" + pd "github.com/tikv/pd/client" ) -type gcv2AbortManager struct { - extworkload.Manager - role config.ExternalWorkloadRole - abortCalled bool -} - -func (m *gcv2AbortManager) Role() config.ExternalWorkloadRole { - return m.role -} -func (m *gcv2AbortManager) AbortGCV2(context.Context) error { - m.abortCalled = true - return nil -} - type gcv2AbortStore struct { kv.Storage - mgr extworkload.Manager + codec tikv.Codec } -func (s *gcv2AbortStore) ExternalWorkloadManager() extworkload.Manager { - return s.mgr +func (s *gcv2AbortStore) GetCodec() tikv.Codec { + return s.codec } -func TestAbortGCV2(t *testing.T) { +func TestAbortGCV2Gate(t *testing.T) { if kerneltype.IsClassic() { t.Skip("Starter deploy mode is only available in nextgen kernel") } originDeployMode := deploymode.Get() - originInTest := intest.InTest + restoreConfig := config.RestoreFunc() t.Cleanup(func() { - intest.InTest = originInTest + restoreConfig() require.NoError(t, deploymode.Set(originDeployMode)) }) + + keyspaceLevelStore := newGCV2AbortStore(t, &keyspacepb.KeyspaceMeta{ + Id: 1, + Name: "ks", + Config: map[string]string{ + pd.KeyspaceConfigGCManagementType: pd.KeyspaceConfigGCManagementTypeKeyspaceLevel, + }, + }) + unifiedGCStore := newGCV2AbortStore(t, &keyspacepb.KeyspaceMeta{ + Id: 2, + Name: "ks2", + Config: map[string]string{ + pd.KeyspaceConfigGCManagementType: pd.KeyspaceConfigGCManagementTypeUnified, + }, + }) + config.UpdateGlobal(func(conf *config.Config) { + conf.ExternalWorkload.Enable = true + conf.ExternalWorkload.Role = config.RoleGCV2Worker + }) + + require.NoError(t, deploymode.Set(deploymode.Premium)) + abortGCV2(keyspaceLevelStore) + require.NoError(t, deploymode.Set(deploymode.Starter)) - intest.InTest = true + config.UpdateGlobal(func(conf *config.Config) { + conf.ExternalWorkload.Role = config.RoleMaster + }) + abortGCV2(keyspaceLevelStore) - mgr := &gcv2AbortManager{role: config.RoleGCV2Worker} - store := &gcv2AbortStore{mgr: mgr} - abortGCV2(store) - require.True(t, mgr.abortCalled) + config.UpdateGlobal(func(conf *config.Config) { + conf.ExternalWorkload.Role = config.RoleGCV2Worker + }) + abortGCV2(unifiedGCStore) +} - mgr.abortCalled = false - mgr.role = config.RoleMaster - abortGCV2(store) - require.False(t, mgr.abortCalled) +func newGCV2AbortStore(t *testing.T, meta *keyspacepb.KeyspaceMeta) *gcv2AbortStore { + codec, err := tikv.NewCodecV2(tikv.ModeTxn, meta) + require.NoError(t, err) + return &gcv2AbortStore{codec: codec} } From afdb3e29392d6d6c3d335d20c69fcdc21ff20f58 Mon Sep 17 00:00:00 2001 From: AmoebaProtozoa <8039876+AmoebaProtozoa@users.noreply.github.com> Date: Tue, 30 Jun 2026 04:52:26 +0800 Subject: [PATCH 07/28] external workload: install GCV2 manager before bootstrap --- cmd/tidb-server/main.go | 26 +++++++-- pkg/session/BUILD.bazel | 3 +- pkg/session/external_workload.go | 26 ++------- pkg/session/external_workload_test.go | 83 ++++++++++++--------------- 4 files changed, 63 insertions(+), 75 deletions(-) diff --git a/cmd/tidb-server/main.go b/cmd/tidb-server/main.go index 135c62e533e66..25136f0bc661f 100644 --- a/cmd/tidb-server/main.go +++ b/cmd/tidb-server/main.go @@ -329,26 +329,35 @@ func initExternalWorkloadManager(ctx context.Context, storage kv.Storage) extwor if !cfg.Enable { return nil } - // Manager init is non-fatal; TiDB can continue without external workload coordination. + // Regular TiDB roles can continue without coordination, but a dedicated + // GCV2 worker must not run without the controller. meta := storage.GetCodec().GetKeyspaceMeta() if meta == nil { + if cfg.Role == config.RoleGCV2Worker { + logutil.BgLogger().Fatal("external workload GCV2 role requires keyspace meta") + } logutil.BgLogger().Warn("external workload controller enabled but keyspace meta is unavailable; TiDB will continue without external workload coordination") return nil } if cfg.Role == config.RoleGCV2Worker && !pd.IsKeyspaceUsingKeyspaceLevelGC(meta) { - logutil.BgLogger().Warn("external workload GCV2 role requires keyspace-level GC; TiDB will continue without external workload coordination") - return nil + logutil.BgLogger().Fatal("external workload GCV2 role requires keyspace-level GC") } mgr, err := extworkload.NewManager(ctx, meta, cfg) if err != nil { + if cfg.Role == config.RoleGCV2Worker { + logutil.BgLogger().Fatal("failed to initialize external workload manager for GCV2 role", zap.Error(err)) + } logutil.BgLogger().Warn("failed to initialize external workload manager; TiDB will continue without external workload coordination", zap.Error(err)) return nil } if !extworkload.SetManagerForStore(storage, mgr) { - logutil.BgLogger().Warn("external workload controller enabled but storage does not support external workload coordination; TiDB will continue without it") if err := mgr.Close(); err != nil { logutil.BgLogger().Warn("failed to close external workload manager", zap.Error(err)) } + if cfg.Role == config.RoleGCV2Worker { + logutil.BgLogger().Fatal("external workload GCV2 role requires storage support") + } + logutil.BgLogger().Warn("external workload controller enabled but storage does not support external workload coordination; TiDB will continue without it") return nil } return mgr @@ -495,13 +504,13 @@ func main() { resourcemanager.InstanceResourceManager.Start() storage, dom, err := createStoreDDLOwnerMgrAndDomain(keyspaceName) terror.MustNil(err) - repository.SetupRepository(dom) - externalWorkloadManager := initExternalWorkloadManager(context.Background(), storage) + externalWorkloadManager := extworkload.GetManagerFromStore(storage) if externalWorkloadManager != nil { defer func() { closeExternalWorkloadManager(storage, externalWorkloadManager) }() } + repository.SetupRepository(dom) if extworkload.IsMaster(externalWorkloadManager) && extworkload.UseKeyspaceLevelGC(externalWorkloadManager) { if err = externalWorkloadManager.InitializeGCV2(context.Background()); err != nil { logutil.BgLogger().Warn("failed to initialize external workload service; TiDB will continue without external workload coordination", zap.Error(err)) @@ -641,17 +650,20 @@ func createStoreDDLOwnerMgrAndDomain(keyspaceName string) (kv.Storage, *domain.D kv.StandAloneTiDB = true } storage := kvstore.MustInitStorage(keyspaceName) + externalWorkloadManager := initExternalWorkloadManager(context.Background(), storage) if tikvStore, ok := storage.(kv.StorageWithPD); ok { pdhttpCli := tikvStore.GetPDHTTPClient() // unistore also implements kv.StorageWithPD, but it does not have PD client. if pdhttpCli != nil { pdStatus, err := pdhttpCli.GetStatus(context.Background()) if err != nil { + closeExternalWorkloadManager(storage, externalWorkloadManager) return nil, nil, err } if !kerneltype.IsMatch(pdStatus.KernelType) { log.Error("kernel type mismatch", zap.String("pd", pdStatus.KernelType), zap.String("tidb", kerneltype.Name())) + closeExternalWorkloadManager(storage, externalWorkloadManager) return nil, nil, errors.New("kernel type mismatch") } } @@ -661,10 +673,12 @@ func createStoreDDLOwnerMgrAndDomain(keyspaceName string) (kv.Storage, *domain.D // Bootstrap a session to load information schema. err := ddl.StartOwnerManager(context.Background(), storage) if err != nil { + closeExternalWorkloadManager(storage, externalWorkloadManager) return nil, nil, err } dom, err := session.BootstrapSession(storage) if err != nil { + closeExternalWorkloadManager(storage, externalWorkloadManager) return nil, nil, err } return storage, dom, nil diff --git a/pkg/session/BUILD.bazel b/pkg/session/BUILD.bazel index 52db78cabfa9e..2cf454ea3ed84 100644 --- a/pkg/session/BUILD.bazel +++ b/pkg/session/BUILD.bazel @@ -148,7 +148,6 @@ go_library( "@com_github_tikv_client_go_v2//trace", "@com_github_tikv_client_go_v2//txnkv/transaction", "@com_github_tikv_client_go_v2//util", - "@com_github_tikv_pd_client//:client", "@io_etcd_go_etcd_client_v3//:client", "@org_uber_go_atomic//:atomic", "@org_uber_go_zap//:zap", @@ -184,6 +183,7 @@ go_test( "//pkg/executor", "//pkg/expression", "//pkg/expression/sessionexpr", + "//pkg/extworkload", "//pkg/keyspace", "//pkg/kv", "//pkg/meta", @@ -218,7 +218,6 @@ go_test( "@com_github_pingcap_log//:log", "@com_github_stretchr_testify//require", "@com_github_tikv_client_go_v2//tikv", - "@com_github_tikv_pd_client//:client", "@io_etcd_go_etcd_tests_v3//integration", "@org_uber_go_atomic//:atomic", #keep "@org_uber_go_goleak//:goleak", diff --git a/pkg/session/external_workload.go b/pkg/session/external_workload.go index b9ce014955f1f..ba216f130e27f 100644 --- a/pkg/session/external_workload.go +++ b/pkg/session/external_workload.go @@ -18,38 +18,20 @@ import ( "context" "os" - "github.com/pingcap/tidb/pkg/config" - "github.com/pingcap/tidb/pkg/config/deploymode" "github.com/pingcap/tidb/pkg/extworkload" "github.com/pingcap/tidb/pkg/kv" "github.com/pingcap/tidb/pkg/util/intest" "github.com/pingcap/tidb/pkg/util/logutil" - pd "github.com/tikv/pd/client" "go.uber.org/zap" ) func abortGCV2(store kv.Storage) { - if !deploymode.IsStarter() { + mgr := extworkload.GetManagerFromStore(store) + if !extworkload.IsGCV2Worker(mgr) { return } - cfg := config.GetGlobalConfig().ExternalWorkload - if !cfg.Enable || cfg.Role != config.RoleGCV2Worker { - return - } - meta := store.GetCodec().GetKeyspaceMeta() - if !pd.IsKeyspaceUsingKeyspaceLevelGC(meta) { - return - } - mgr, err := extworkload.NewManager(context.Background(), meta, cfg) - if err != nil { - logutil.BgLogger().Fatal("initialize external workload manager for GCV2 abort failed", zap.Error(err)) - } - abortErr := mgr.AbortGCV2(context.Background()) - if err := mgr.Close(); err != nil { - logutil.BgLogger().Warn("failed to close external workload manager after GCV2 abort", zap.Error(err)) - } - if abortErr != nil { - logutil.BgLogger().Fatal("abort GCV2 worker failed", zap.Error(abortErr)) + if err := mgr.AbortGCV2(context.Background()); err != nil { + logutil.BgLogger().Fatal("abort GCV2 worker failed", zap.Error(err)) } logutil.BgLogger().Info("GCV2 worker aborted") if intest.InTest { diff --git a/pkg/session/external_workload_test.go b/pkg/session/external_workload_test.go index e984e1c3376c5..a4321e29d4e0f 100644 --- a/pkg/session/external_workload_test.go +++ b/pkg/session/external_workload_test.go @@ -15,75 +15,68 @@ package session import ( + "context" "testing" - "github.com/pingcap/kvproto/pkg/keyspacepb" "github.com/pingcap/tidb/pkg/config" "github.com/pingcap/tidb/pkg/config/deploymode" "github.com/pingcap/tidb/pkg/config/kerneltype" + "github.com/pingcap/tidb/pkg/extworkload" "github.com/pingcap/tidb/pkg/kv" + "github.com/pingcap/tidb/pkg/util/intest" "github.com/stretchr/testify/require" - "github.com/tikv/client-go/v2/tikv" - pd "github.com/tikv/pd/client" ) +type gcv2AbortManager struct { + extworkload.Manager + role config.ExternalWorkloadRole + abortCalled bool +} + +func (m *gcv2AbortManager) Role() config.ExternalWorkloadRole { + return m.role +} + +func (m *gcv2AbortManager) AbortGCV2(context.Context) error { + m.abortCalled = true + return nil +} + type gcv2AbortStore struct { kv.Storage - codec tikv.Codec + mgr extworkload.Manager } -func (s *gcv2AbortStore) GetCodec() tikv.Codec { - return s.codec +func (s *gcv2AbortStore) ExternalWorkloadManager() extworkload.Manager { + return s.mgr } -func TestAbortGCV2Gate(t *testing.T) { +func TestAbortGCV2(t *testing.T) { if kerneltype.IsClassic() { t.Skip("Starter deploy mode is only available in nextgen kernel") } originDeployMode := deploymode.Get() - restoreConfig := config.RestoreFunc() + originInTest := intest.InTest t.Cleanup(func() { - restoreConfig() + intest.InTest = originInTest require.NoError(t, deploymode.Set(originDeployMode)) }) - - keyspaceLevelStore := newGCV2AbortStore(t, &keyspacepb.KeyspaceMeta{ - Id: 1, - Name: "ks", - Config: map[string]string{ - pd.KeyspaceConfigGCManagementType: pd.KeyspaceConfigGCManagementTypeKeyspaceLevel, - }, - }) - unifiedGCStore := newGCV2AbortStore(t, &keyspacepb.KeyspaceMeta{ - Id: 2, - Name: "ks2", - Config: map[string]string{ - pd.KeyspaceConfigGCManagementType: pd.KeyspaceConfigGCManagementTypeUnified, - }, - }) - config.UpdateGlobal(func(conf *config.Config) { - conf.ExternalWorkload.Enable = true - conf.ExternalWorkload.Role = config.RoleGCV2Worker - }) - - require.NoError(t, deploymode.Set(deploymode.Premium)) - abortGCV2(keyspaceLevelStore) - require.NoError(t, deploymode.Set(deploymode.Starter)) - config.UpdateGlobal(func(conf *config.Config) { - conf.ExternalWorkload.Role = config.RoleMaster - }) - abortGCV2(keyspaceLevelStore) + intest.InTest = true - config.UpdateGlobal(func(conf *config.Config) { - conf.ExternalWorkload.Role = config.RoleGCV2Worker - }) - abortGCV2(unifiedGCStore) -} + mgr := &gcv2AbortManager{role: config.RoleGCV2Worker} + store := &gcv2AbortStore{mgr: mgr} + abortGCV2(store) + require.True(t, mgr.abortCalled) -func newGCV2AbortStore(t *testing.T, meta *keyspacepb.KeyspaceMeta) *gcv2AbortStore { - codec, err := tikv.NewCodecV2(tikv.ModeTxn, meta) - require.NoError(t, err) - return &gcv2AbortStore{codec: codec} + mgr.abortCalled = false + mgr.role = config.RoleMaster + abortGCV2(store) + require.False(t, mgr.abortCalled) + + mgr.role = config.RoleGCV2Worker + require.NoError(t, deploymode.Set(deploymode.Premium)) + abortGCV2(store) + require.False(t, mgr.abortCalled) } From a544f8f02aa4208aac2660d138579243e48b3597 Mon Sep 17 00:00:00 2001 From: AmoebaProtozoa <8039876+AmoebaProtozoa@users.noreply.github.com> Date: Tue, 30 Jun 2026 04:59:09 +0800 Subject: [PATCH 08/28] external workload: trim GCV2 session test --- cmd/tidb-server/main.go | 2 +- pkg/session/BUILD.bazel | 3 - pkg/session/external_workload_test.go | 82 --------------------------- 3 files changed, 1 insertion(+), 86 deletions(-) delete mode 100644 pkg/session/external_workload_test.go diff --git a/cmd/tidb-server/main.go b/cmd/tidb-server/main.go index 25136f0bc661f..6b22a748cb195 100644 --- a/cmd/tidb-server/main.go +++ b/cmd/tidb-server/main.go @@ -504,13 +504,13 @@ func main() { resourcemanager.InstanceResourceManager.Start() storage, dom, err := createStoreDDLOwnerMgrAndDomain(keyspaceName) terror.MustNil(err) + repository.SetupRepository(dom) externalWorkloadManager := extworkload.GetManagerFromStore(storage) if externalWorkloadManager != nil { defer func() { closeExternalWorkloadManager(storage, externalWorkloadManager) }() } - repository.SetupRepository(dom) if extworkload.IsMaster(externalWorkloadManager) && extworkload.UseKeyspaceLevelGC(externalWorkloadManager) { if err = externalWorkloadManager.InitializeGCV2(context.Background()); err != nil { logutil.BgLogger().Warn("failed to initialize external workload service; TiDB will continue without external workload coordination", zap.Error(err)) diff --git a/pkg/session/BUILD.bazel b/pkg/session/BUILD.bazel index 2cf454ea3ed84..b897c78801be6 100644 --- a/pkg/session/BUILD.bazel +++ b/pkg/session/BUILD.bazel @@ -161,7 +161,6 @@ go_test( srcs = [ "bench_test.go", "bootstrap_test.go", - "external_workload_test.go", "main_test.go", "session_test.go", "tidb_test.go", @@ -176,14 +175,12 @@ go_test( "//pkg/autoid_service", "//pkg/bindinfo", "//pkg/config", - "//pkg/config/deploymode", "//pkg/config/kerneltype", "//pkg/ddl", "//pkg/domain", "//pkg/executor", "//pkg/expression", "//pkg/expression/sessionexpr", - "//pkg/extworkload", "//pkg/keyspace", "//pkg/kv", "//pkg/meta", diff --git a/pkg/session/external_workload_test.go b/pkg/session/external_workload_test.go deleted file mode 100644 index a4321e29d4e0f..0000000000000 --- a/pkg/session/external_workload_test.go +++ /dev/null @@ -1,82 +0,0 @@ -// Copyright 2026 PingCAP, Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package session - -import ( - "context" - "testing" - - "github.com/pingcap/tidb/pkg/config" - "github.com/pingcap/tidb/pkg/config/deploymode" - "github.com/pingcap/tidb/pkg/config/kerneltype" - "github.com/pingcap/tidb/pkg/extworkload" - "github.com/pingcap/tidb/pkg/kv" - "github.com/pingcap/tidb/pkg/util/intest" - "github.com/stretchr/testify/require" -) - -type gcv2AbortManager struct { - extworkload.Manager - role config.ExternalWorkloadRole - abortCalled bool -} - -func (m *gcv2AbortManager) Role() config.ExternalWorkloadRole { - return m.role -} - -func (m *gcv2AbortManager) AbortGCV2(context.Context) error { - m.abortCalled = true - return nil -} - -type gcv2AbortStore struct { - kv.Storage - mgr extworkload.Manager -} - -func (s *gcv2AbortStore) ExternalWorkloadManager() extworkload.Manager { - return s.mgr -} - -func TestAbortGCV2(t *testing.T) { - if kerneltype.IsClassic() { - t.Skip("Starter deploy mode is only available in nextgen kernel") - } - - originDeployMode := deploymode.Get() - originInTest := intest.InTest - t.Cleanup(func() { - intest.InTest = originInTest - require.NoError(t, deploymode.Set(originDeployMode)) - }) - require.NoError(t, deploymode.Set(deploymode.Starter)) - intest.InTest = true - - mgr := &gcv2AbortManager{role: config.RoleGCV2Worker} - store := &gcv2AbortStore{mgr: mgr} - abortGCV2(store) - require.True(t, mgr.abortCalled) - - mgr.abortCalled = false - mgr.role = config.RoleMaster - abortGCV2(store) - require.False(t, mgr.abortCalled) - - mgr.role = config.RoleGCV2Worker - require.NoError(t, deploymode.Set(deploymode.Premium)) - abortGCV2(store) - require.False(t, mgr.abortCalled) -} From 51df1b2aba6252c6d6bce1be0997cf805fc0bb8f Mon Sep 17 00:00:00 2001 From: AmoebaProtozoa <8039876+AmoebaProtozoa@users.noreply.github.com> Date: Tue, 30 Jun 2026 05:03:28 +0800 Subject: [PATCH 09/28] external workload: keep store setter local --- cmd/tidb-server/main.go | 12 ++++++++++-- pkg/extworkload/util.go | 30 ++++++++---------------------- pkg/extworkload/util_test.go | 9 ++------- 3 files changed, 20 insertions(+), 31 deletions(-) diff --git a/cmd/tidb-server/main.go b/cmd/tidb-server/main.go index 6b22a748cb195..a18535ded4ccc 100644 --- a/cmd/tidb-server/main.go +++ b/cmd/tidb-server/main.go @@ -350,7 +350,8 @@ func initExternalWorkloadManager(ctx context.Context, storage kv.Storage) extwor logutil.BgLogger().Warn("failed to initialize external workload manager; TiDB will continue without external workload coordination", zap.Error(err)) return nil } - if !extworkload.SetManagerForStore(storage, mgr) { + managerStore, ok := storage.(externalWorkloadManagerSetter) + if !ok { if err := mgr.Close(); err != nil { logutil.BgLogger().Warn("failed to close external workload manager", zap.Error(err)) } @@ -360,14 +361,21 @@ func initExternalWorkloadManager(ctx context.Context, storage kv.Storage) extwor logutil.BgLogger().Warn("external workload controller enabled but storage does not support external workload coordination; TiDB will continue without it") return nil } + managerStore.SetExternalWorkloadManager(mgr) return mgr } +type externalWorkloadManagerSetter interface { + SetExternalWorkloadManager(extworkload.Manager) +} + func closeExternalWorkloadManager(storage kv.Storage, mgr extworkload.Manager) { if mgr == nil { return } - extworkload.SetManagerForStore(storage, nil) + if managerStore, ok := storage.(externalWorkloadManagerSetter); ok { + managerStore.SetExternalWorkloadManager(nil) + } if err := mgr.Close(); err != nil { logutil.BgLogger().Warn("failed to close external workload manager", zap.Error(err)) } diff --git a/pkg/extworkload/util.go b/pkg/extworkload/util.go index bca51d919bdc7..e83e043228e02 100644 --- a/pkg/extworkload/util.go +++ b/pkg/extworkload/util.go @@ -37,29 +37,11 @@ func IsAutoAnalyzeWorker(m Manager) bool { return roleIs(m, config.RoleAutoAnaly // UseKeyspaceLevelGC reports whether the manager is bound to a keyspace-level GC keyspace. func UseKeyspaceLevelGC(m Manager) bool { - return IsEnabled(m) && m.Meta() != nil && pd.IsKeyspaceUsingKeyspaceLevelGC(m.Meta()) -} - -// ManagerProvider is implemented by storage instances that carry an external -// workload manager. -type ManagerProvider interface { - ExternalWorkloadManager() Manager -} - -// ManagerSetter is implemented by storage instances that can install an -// external workload manager. -type ManagerSetter interface { - SetExternalWorkloadManager(Manager) -} - -// SetManagerForStore installs the manager on a storage instance that supports it. -func SetManagerForStore(store any, mgr Manager) bool { - setter, ok := store.(ManagerSetter) - if !ok { + if !IsEnabled(m) { return false } - setter.SetExternalWorkloadManager(mgr) - return true + meta := m.Meta() + return meta != nil && pd.IsKeyspaceUsingKeyspaceLevelGC(meta) } // GetManagerFromStore returns the manager bound to store only in Starter deploy mode. @@ -67,13 +49,17 @@ func GetManagerFromStore(store any) Manager { if !deploymode.IsStarter() { return nil } - provider, ok := store.(ManagerProvider) + provider, ok := store.(managerProvider) if !ok { return nil } return provider.ExternalWorkloadManager() } +type managerProvider interface { + ExternalWorkloadManager() Manager +} + func roleIs(m Manager, role config.ExternalWorkloadRole) bool { return IsEnabled(m) && m.Role() == role } diff --git a/pkg/extworkload/util_test.go b/pkg/extworkload/util_test.go index 2558f592f4f8a..e59eb14621c12 100644 --- a/pkg/extworkload/util_test.go +++ b/pkg/extworkload/util_test.go @@ -38,10 +38,6 @@ type stubManagerStore struct { mgr Manager } -func (s *stubManagerStore) SetExternalWorkloadManager(mgr Manager) { - s.mgr = mgr -} - func (s *stubManagerStore) ExternalWorkloadManager() Manager { return s.mgr } @@ -100,14 +96,13 @@ func TestManagerFromStoreStarterGate(t *testing.T) { }) mgr := &stubManager{role: config.RoleMaster} - store := &stubManagerStore{} - require.True(t, SetManagerForStore(store, mgr)) + store := &stubManagerStore{mgr: mgr} require.NoError(t, deploymode.Set(deploymode.Premium)) require.Nil(t, GetManagerFromStore(store)) require.NoError(t, deploymode.Set(deploymode.Starter)) require.Same(t, mgr, GetManagerFromStore(store)) - require.True(t, SetManagerForStore(store, nil)) + store.mgr = nil require.Nil(t, GetManagerFromStore(store)) } From da94530b6847ecc2e707ae6941222748dd378532 Mon Sep 17 00:00:00 2001 From: AmoebaProtozoa <8039876+AmoebaProtozoa@users.noreply.github.com> Date: Tue, 30 Jun 2026 05:06:30 +0800 Subject: [PATCH 10/28] external workload: inline keyspace GC checks --- cmd/tidb-server/main.go | 2 +- pkg/executor/set.go | 3 ++- pkg/extworkload/BUILD.bazel | 1 - pkg/extworkload/util.go | 10 ---------- pkg/extworkload/util_test.go | 18 ------------------ pkg/store/gcworker/gc_worker.go | 2 +- 6 files changed, 4 insertions(+), 32 deletions(-) diff --git a/cmd/tidb-server/main.go b/cmd/tidb-server/main.go index a18535ded4ccc..dc332cc811148 100644 --- a/cmd/tidb-server/main.go +++ b/cmd/tidb-server/main.go @@ -519,7 +519,7 @@ func main() { closeExternalWorkloadManager(storage, externalWorkloadManager) }() } - if extworkload.IsMaster(externalWorkloadManager) && extworkload.UseKeyspaceLevelGC(externalWorkloadManager) { + if extworkload.IsMaster(externalWorkloadManager) && pd.IsKeyspaceUsingKeyspaceLevelGC(externalWorkloadManager.Meta()) { if err = externalWorkloadManager.InitializeGCV2(context.Background()); err != nil { logutil.BgLogger().Warn("failed to initialize external workload service; TiDB will continue without external workload coordination", zap.Error(err)) closeExternalWorkloadManager(storage, externalWorkloadManager) diff --git a/pkg/executor/set.go b/pkg/executor/set.go index f68a7ee7b8c1d..f5391c9644f51 100644 --- a/pkg/executor/set.go +++ b/pkg/executor/set.go @@ -44,6 +44,7 @@ import ( sem "github.com/pingcap/tidb/pkg/util/sem/compat" semv2 "github.com/pingcap/tidb/pkg/util/sem/v2" "github.com/tikv/client-go/v2/oracle/oracles" + pd "github.com/tikv/pd/client" "go.uber.org/zap" ) @@ -193,7 +194,7 @@ func (e *SetExecutor) setSysVariable(ctx context.Context, name string, v *expres logutil.BgLogger().Info(logstr, zap.Uint64("conn", sessionVars.ConnectionID), zap.String("name", name), zap.String("val", showValStr)) if v.IsGlobal && name == vardef.TiDBGCLifetime { mgr := extworkload.GetManagerFromStore(e.Ctx().GetStore()) - if extworkload.IsMaster(mgr) && extworkload.UseKeyspaceLevelGC(mgr) { + if extworkload.IsMaster(mgr) && pd.IsKeyspaceUsingKeyspaceLevelGC(mgr.Meta()) { gcLifeTime, err := time.ParseDuration(valStr) if err != nil { return err diff --git a/pkg/extworkload/BUILD.bazel b/pkg/extworkload/BUILD.bazel index 733833e22140c..a583c53078a5b 100644 --- a/pkg/extworkload/BUILD.bazel +++ b/pkg/extworkload/BUILD.bazel @@ -17,7 +17,6 @@ go_library( "//pkg/util/logutil", "@com_github_pingcap_errors//:errors", "@com_github_pingcap_kvproto//pkg/keyspacepb", - "@com_github_tikv_pd_client//:client", "@org_golang_google_grpc//:grpc", "@org_uber_go_zap//:zap", ], diff --git a/pkg/extworkload/util.go b/pkg/extworkload/util.go index e83e043228e02..a2185adad9cb3 100644 --- a/pkg/extworkload/util.go +++ b/pkg/extworkload/util.go @@ -17,7 +17,6 @@ package extworkload import ( "github.com/pingcap/tidb/pkg/config" "github.com/pingcap/tidb/pkg/config/deploymode" - pd "github.com/tikv/pd/client" ) // IsEnabled reports whether a Manager is present. @@ -35,15 +34,6 @@ func IsTTLTaskWorker(m Manager) bool { return roleIs(m, config.RoleTTLTaskWorker // IsAutoAnalyzeWorker reports whether this TiDB should run auto-analyze jobs. func IsAutoAnalyzeWorker(m Manager) bool { return roleIs(m, config.RoleAutoAnalyzeWorker) } -// UseKeyspaceLevelGC reports whether the manager is bound to a keyspace-level GC keyspace. -func UseKeyspaceLevelGC(m Manager) bool { - if !IsEnabled(m) { - return false - } - meta := m.Meta() - return meta != nil && pd.IsKeyspaceUsingKeyspaceLevelGC(meta) -} - // GetManagerFromStore returns the manager bound to store only in Starter deploy mode. func GetManagerFromStore(store any) Manager { if !deploymode.IsStarter() { diff --git a/pkg/extworkload/util_test.go b/pkg/extworkload/util_test.go index e59eb14621c12..729180fe59b30 100644 --- a/pkg/extworkload/util_test.go +++ b/pkg/extworkload/util_test.go @@ -17,22 +17,18 @@ package extworkload import ( "testing" - "github.com/pingcap/kvproto/pkg/keyspacepb" "github.com/pingcap/tidb/pkg/config" "github.com/pingcap/tidb/pkg/config/deploymode" "github.com/pingcap/tidb/pkg/config/kerneltype" "github.com/stretchr/testify/require" - pd "github.com/tikv/pd/client" ) type stubManager struct { Manager role config.ExternalWorkloadRole - meta *keyspacepb.KeyspaceMeta } func (s *stubManager) Role() config.ExternalWorkloadRole { return s.role } -func (s *stubManager) Meta() *keyspacepb.KeyspaceMeta { return s.meta } type stubManagerStore struct { mgr Manager @@ -71,20 +67,6 @@ func TestRolePredicatesDedicated(t *testing.T) { } } -func TestUseKeyspaceLevelGC(t *testing.T) { - mgr := &stubManager{ - role: config.RoleMaster, - } - require.False(t, UseKeyspaceLevelGC(mgr)) - - mgr.meta = &keyspacepb.KeyspaceMeta{ - Id: 1, - Name: "ks", - Config: map[string]string{pd.KeyspaceConfigGCManagementType: pd.KeyspaceConfigGCManagementTypeKeyspaceLevel}, - } - require.True(t, UseKeyspaceLevelGC(mgr)) -} - func TestManagerFromStoreStarterGate(t *testing.T) { if kerneltype.IsClassic() { t.Skip("Starter deploy mode is only available in nextgen kernel") diff --git a/pkg/store/gcworker/gc_worker.go b/pkg/store/gcworker/gc_worker.go index 374de72a60324..7ace9f2f4a3a2 100644 --- a/pkg/store/gcworker/gc_worker.go +++ b/pkg/store/gcworker/gc_worker.go @@ -850,7 +850,7 @@ func (w *GCWorker) runGCJob(ctx context.Context, safePoint uint64, concurrency g func (w *GCWorker) notifyGCV2AfterGC(ctx context.Context, safePoint uint64) error { mgr := extworkload.GetManagerFromStore(w.store) - if !extworkload.UseKeyspaceLevelGC(mgr) { + if !extworkload.IsEnabled(mgr) || !pd.IsKeyspaceUsingKeyspaceLevelGC(mgr.Meta()) { return nil } From 268aac79fa68cffe3270f6dc7fbfb9ae057139c5 Mon Sep 17 00:00:00 2001 From: AmoebaProtozoa <8039876+AmoebaProtozoa@users.noreply.github.com> Date: Tue, 30 Jun 2026 05:09:07 +0800 Subject: [PATCH 11/28] external workload: flatten role helpers --- pkg/extworkload/util.go | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/pkg/extworkload/util.go b/pkg/extworkload/util.go index a2185adad9cb3..e6f3fdb5be09c 100644 --- a/pkg/extworkload/util.go +++ b/pkg/extworkload/util.go @@ -23,33 +23,29 @@ import ( func IsEnabled(m Manager) bool { return m != nil } // IsMaster reports whether this TiDB is in the regular TiDB role. -func IsMaster(m Manager) bool { return roleIs(m, config.RoleMaster) } +func IsMaster(m Manager) bool { return IsEnabled(m) && m.Role() == config.RoleMaster } // IsGCV2Worker reports whether this TiDB is a dedicated keyspace-level GC worker. -func IsGCV2Worker(m Manager) bool { return roleIs(m, config.RoleGCV2Worker) } +func IsGCV2Worker(m Manager) bool { return IsEnabled(m) && m.Role() == config.RoleGCV2Worker } // IsTTLTaskWorker reports whether this TiDB should run TTL jobs. -func IsTTLTaskWorker(m Manager) bool { return roleIs(m, config.RoleTTLTaskWorker) } +func IsTTLTaskWorker(m Manager) bool { return IsEnabled(m) && m.Role() == config.RoleTTLTaskWorker } // IsAutoAnalyzeWorker reports whether this TiDB should run auto-analyze jobs. -func IsAutoAnalyzeWorker(m Manager) bool { return roleIs(m, config.RoleAutoAnalyzeWorker) } +func IsAutoAnalyzeWorker(m Manager) bool { + return IsEnabled(m) && m.Role() == config.RoleAutoAnalyzeWorker +} // GetManagerFromStore returns the manager bound to store only in Starter deploy mode. func GetManagerFromStore(store any) Manager { if !deploymode.IsStarter() { return nil } - provider, ok := store.(managerProvider) + provider, ok := store.(interface { + ExternalWorkloadManager() Manager + }) if !ok { return nil } return provider.ExternalWorkloadManager() } - -type managerProvider interface { - ExternalWorkloadManager() Manager -} - -func roleIs(m Manager, role config.ExternalWorkloadRole) bool { - return IsEnabled(m) && m.Role() == role -} From 75ede41c174dc05240f9f6b9a05c5406c2f33e91 Mon Sep 17 00:00:00 2001 From: AmoebaProtozoa <8039876+AmoebaProtozoa@users.noreply.github.com> Date: Tue, 30 Jun 2026 05:12:29 +0800 Subject: [PATCH 12/28] external workload: store manager in kv options --- cmd/tidb-server/main.go | 21 ++------------------- pkg/extworkload/BUILD.bazel | 3 +-- pkg/extworkload/util.go | 23 ++++++++++++++++++----- pkg/extworkload/util_test.go | 32 -------------------------------- pkg/store/driver/BUILD.bazel | 1 - pkg/store/driver/tikv_driver.go | 19 ------------------- 6 files changed, 21 insertions(+), 78 deletions(-) diff --git a/cmd/tidb-server/main.go b/cmd/tidb-server/main.go index dc332cc811148..b2d665eb99ecb 100644 --- a/cmd/tidb-server/main.go +++ b/cmd/tidb-server/main.go @@ -350,32 +350,15 @@ func initExternalWorkloadManager(ctx context.Context, storage kv.Storage) extwor logutil.BgLogger().Warn("failed to initialize external workload manager; TiDB will continue without external workload coordination", zap.Error(err)) return nil } - managerStore, ok := storage.(externalWorkloadManagerSetter) - if !ok { - if err := mgr.Close(); err != nil { - logutil.BgLogger().Warn("failed to close external workload manager", zap.Error(err)) - } - if cfg.Role == config.RoleGCV2Worker { - logutil.BgLogger().Fatal("external workload GCV2 role requires storage support") - } - logutil.BgLogger().Warn("external workload controller enabled but storage does not support external workload coordination; TiDB will continue without it") - return nil - } - managerStore.SetExternalWorkloadManager(mgr) + extworkload.SetManagerForStore(storage, mgr) return mgr } -type externalWorkloadManagerSetter interface { - SetExternalWorkloadManager(extworkload.Manager) -} - func closeExternalWorkloadManager(storage kv.Storage, mgr extworkload.Manager) { if mgr == nil { return } - if managerStore, ok := storage.(externalWorkloadManagerSetter); ok { - managerStore.SetExternalWorkloadManager(nil) - } + extworkload.SetManagerForStore(storage, nil) if err := mgr.Close(); err != nil { logutil.BgLogger().Warn("failed to close external workload manager", zap.Error(err)) } diff --git a/pkg/extworkload/BUILD.bazel b/pkg/extworkload/BUILD.bazel index a583c53078a5b..e10f73e1bfa43 100644 --- a/pkg/extworkload/BUILD.bazel +++ b/pkg/extworkload/BUILD.bazel @@ -13,6 +13,7 @@ go_library( "//pkg/config", "//pkg/config/deploymode", "//pkg/extworkload/client", + "//pkg/kv", "//pkg/metrics", "//pkg/util/logutil", "@com_github_pingcap_errors//:errors", @@ -34,8 +35,6 @@ go_test( shard_count = 6, deps = [ "//pkg/config", - "//pkg/config/deploymode", - "//pkg/config/kerneltype", "//pkg/extworkload/client", "//pkg/metrics", "@com_github_pingcap_kvproto//pkg/externalworkloadpb", diff --git a/pkg/extworkload/util.go b/pkg/extworkload/util.go index e6f3fdb5be09c..ab8662f5c8ee3 100644 --- a/pkg/extworkload/util.go +++ b/pkg/extworkload/util.go @@ -17,6 +17,7 @@ package extworkload import ( "github.com/pingcap/tidb/pkg/config" "github.com/pingcap/tidb/pkg/config/deploymode" + "github.com/pingcap/tidb/pkg/kv" ) // IsEnabled reports whether a Manager is present. @@ -36,16 +37,28 @@ func IsAutoAnalyzeWorker(m Manager) bool { return IsEnabled(m) && m.Role() == config.RoleAutoAnalyzeWorker } +type managerStoreKey struct{} + +// SetManagerForStore binds the manager to store. Passing nil removes it. +func SetManagerForStore(store kv.Storage, mgr Manager) { + if store == nil { + return + } + store.SetOption(managerStoreKey{}, mgr) +} + // GetManagerFromStore returns the manager bound to store only in Starter deploy mode. -func GetManagerFromStore(store any) Manager { +func GetManagerFromStore(store kv.Storage) Manager { if !deploymode.IsStarter() { return nil } - provider, ok := store.(interface { - ExternalWorkloadManager() Manager - }) + if store == nil { + return nil + } + v, ok := store.GetOption(managerStoreKey{}) if !ok { return nil } - return provider.ExternalWorkloadManager() + mgr, _ := v.(Manager) + return mgr } diff --git a/pkg/extworkload/util_test.go b/pkg/extworkload/util_test.go index 729180fe59b30..643932a9bd59a 100644 --- a/pkg/extworkload/util_test.go +++ b/pkg/extworkload/util_test.go @@ -18,8 +18,6 @@ import ( "testing" "github.com/pingcap/tidb/pkg/config" - "github.com/pingcap/tidb/pkg/config/deploymode" - "github.com/pingcap/tidb/pkg/config/kerneltype" "github.com/stretchr/testify/require" ) @@ -30,14 +28,6 @@ type stubManager struct { func (s *stubManager) Role() config.ExternalWorkloadRole { return s.role } -type stubManagerStore struct { - mgr Manager -} - -func (s *stubManagerStore) ExternalWorkloadManager() Manager { - return s.mgr -} - func TestRolePredicatesWhenDisabled(t *testing.T) { require.False(t, IsEnabled(nil)) require.False(t, IsMaster(nil)) @@ -66,25 +56,3 @@ func TestRolePredicatesDedicated(t *testing.T) { }) } } - -func TestManagerFromStoreStarterGate(t *testing.T) { - if kerneltype.IsClassic() { - t.Skip("Starter deploy mode is only available in nextgen kernel") - } - - origin := deploymode.Get() - t.Cleanup(func() { - require.NoError(t, deploymode.Set(origin)) - }) - - mgr := &stubManager{role: config.RoleMaster} - store := &stubManagerStore{mgr: mgr} - - require.NoError(t, deploymode.Set(deploymode.Premium)) - require.Nil(t, GetManagerFromStore(store)) - - require.NoError(t, deploymode.Set(deploymode.Starter)) - require.Same(t, mgr, GetManagerFromStore(store)) - store.mgr = nil - require.Nil(t, GetManagerFromStore(store)) -} diff --git a/pkg/store/driver/BUILD.bazel b/pkg/store/driver/BUILD.bazel index af1ce3a22ca93..815d866b9d96a 100644 --- a/pkg/store/driver/BUILD.bazel +++ b/pkg/store/driver/BUILD.bazel @@ -10,7 +10,6 @@ go_library( "//pkg/metaservice", "//pkg/metrics", "//pkg/metrics/common", - "//pkg/extworkload", "//pkg/sessionctx/variable", "//pkg/store/copr", "//pkg/store/driver/error", diff --git a/pkg/store/driver/tikv_driver.go b/pkg/store/driver/tikv_driver.go index 8365bb2307049..4e5d4601053c9 100644 --- a/pkg/store/driver/tikv_driver.go +++ b/pkg/store/driver/tikv_driver.go @@ -26,7 +26,6 @@ import ( "github.com/pingcap/errors" deadlockpb "github.com/pingcap/kvproto/pkg/deadlock" "github.com/pingcap/kvproto/pkg/kvrpcpb" - "github.com/pingcap/tidb/pkg/extworkload" "github.com/pingcap/tidb/pkg/kv" "github.com/pingcap/tidb/pkg/metaservice" "github.com/pingcap/tidb/pkg/metrics" @@ -321,27 +320,9 @@ type tikvStore struct { keyspace string closed bool - externalWorkload struct { - sync.RWMutex - mgr extworkload.Manager - } metaServiceInfo *metaservice.Info } -// SetExternalWorkloadManager installs the external workload manager bound to this store. -func (s *tikvStore) SetExternalWorkloadManager(mgr extworkload.Manager) { - s.externalWorkload.Lock() - s.externalWorkload.mgr = mgr - s.externalWorkload.Unlock() -} - -// ExternalWorkloadManager returns the external workload manager bound to this store. -func (s *tikvStore) ExternalWorkloadManager() extworkload.Manager { - s.externalWorkload.RLock() - defer s.externalWorkload.RUnlock() - return s.externalWorkload.mgr -} - // GetOption wraps around sync.Map. func (s *tikvStore) GetOption(k any) (any, bool) { return s.opts.Load(k) From 75709473a0b10124ed62eaf68b76ae3895b92de2 Mon Sep 17 00:00:00 2001 From: AmoebaProtozoa <8039876+AmoebaProtozoa@users.noreply.github.com> Date: Tue, 30 Jun 2026 05:17:44 +0800 Subject: [PATCH 13/28] external workload: preserve role predicate helper --- pkg/extworkload/util.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/pkg/extworkload/util.go b/pkg/extworkload/util.go index ab8662f5c8ee3..83a2b78f58b27 100644 --- a/pkg/extworkload/util.go +++ b/pkg/extworkload/util.go @@ -24,18 +24,16 @@ import ( func IsEnabled(m Manager) bool { return m != nil } // IsMaster reports whether this TiDB is in the regular TiDB role. -func IsMaster(m Manager) bool { return IsEnabled(m) && m.Role() == config.RoleMaster } +func IsMaster(m Manager) bool { return roleIs(m, config.RoleMaster) } // IsGCV2Worker reports whether this TiDB is a dedicated keyspace-level GC worker. -func IsGCV2Worker(m Manager) bool { return IsEnabled(m) && m.Role() == config.RoleGCV2Worker } +func IsGCV2Worker(m Manager) bool { return roleIs(m, config.RoleGCV2Worker) } // IsTTLTaskWorker reports whether this TiDB should run TTL jobs. -func IsTTLTaskWorker(m Manager) bool { return IsEnabled(m) && m.Role() == config.RoleTTLTaskWorker } +func IsTTLTaskWorker(m Manager) bool { return roleIs(m, config.RoleTTLTaskWorker) } // IsAutoAnalyzeWorker reports whether this TiDB should run auto-analyze jobs. -func IsAutoAnalyzeWorker(m Manager) bool { - return IsEnabled(m) && m.Role() == config.RoleAutoAnalyzeWorker -} +func IsAutoAnalyzeWorker(m Manager) bool { return roleIs(m, config.RoleAutoAnalyzeWorker) } type managerStoreKey struct{} @@ -62,3 +60,7 @@ func GetManagerFromStore(store kv.Storage) Manager { mgr, _ := v.(Manager) return mgr } + +func roleIs(m Manager, role config.ExternalWorkloadRole) bool { + return IsEnabled(m) && m.Role() == role +} From d76cf6b55e8d814ffdad70c1d314c22912303d49 Mon Sep 17 00:00:00 2001 From: AmoebaProtozoa <8039876+AmoebaProtozoa@users.noreply.github.com> Date: Tue, 30 Jun 2026 05:29:06 +0800 Subject: [PATCH 14/28] tidb-server: return external workload manager from bootstrap --- cmd/tidb-server/main.go | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/cmd/tidb-server/main.go b/cmd/tidb-server/main.go index b2d665eb99ecb..02222ff13c264 100644 --- a/cmd/tidb-server/main.go +++ b/cmd/tidb-server/main.go @@ -493,10 +493,9 @@ func main() { keyspaceName := keyspace.GetKeyspaceNameBySettings() executor.Start() resourcemanager.InstanceResourceManager.Start() - storage, dom, err := createStoreDDLOwnerMgrAndDomain(keyspaceName) + storage, dom, externalWorkloadManager, err := createStoreDDLOwnerMgrAndDomain(keyspaceName) terror.MustNil(err) repository.SetupRepository(dom) - externalWorkloadManager := extworkload.GetManagerFromStore(storage) if externalWorkloadManager != nil { defer func() { closeExternalWorkloadManager(storage, externalWorkloadManager) @@ -636,7 +635,7 @@ func registerStores() error { return err } -func createStoreDDLOwnerMgrAndDomain(keyspaceName string) (kv.Storage, *domain.Domain, error) { +func createStoreDDLOwnerMgrAndDomain(keyspaceName string) (kv.Storage, *domain.Domain, extworkload.Manager, error) { if config.GetGlobalConfig().Store == config.StoreTypeUniStore { kv.StandAloneTiDB = true } @@ -649,13 +648,13 @@ func createStoreDDLOwnerMgrAndDomain(keyspaceName string) (kv.Storage, *domain.D pdStatus, err := pdhttpCli.GetStatus(context.Background()) if err != nil { closeExternalWorkloadManager(storage, externalWorkloadManager) - return nil, nil, err + return nil, nil, nil, err } if !kerneltype.IsMatch(pdStatus.KernelType) { log.Error("kernel type mismatch", zap.String("pd", pdStatus.KernelType), zap.String("tidb", kerneltype.Name())) closeExternalWorkloadManager(storage, externalWorkloadManager) - return nil, nil, errors.New("kernel type mismatch") + return nil, nil, nil, errors.New("kernel type mismatch") } } } @@ -665,14 +664,14 @@ func createStoreDDLOwnerMgrAndDomain(keyspaceName string) (kv.Storage, *domain.D err := ddl.StartOwnerManager(context.Background(), storage) if err != nil { closeExternalWorkloadManager(storage, externalWorkloadManager) - return nil, nil, err + return nil, nil, nil, err } dom, err := session.BootstrapSession(storage) if err != nil { closeExternalWorkloadManager(storage, externalWorkloadManager) - return nil, nil, err + return nil, nil, nil, err } - return storage, dom, nil + return storage, dom, externalWorkloadManager, nil } // Prometheus push. From d260bb8cfb43e7697528f144fdaa0d5237c68a0e Mon Sep 17 00:00:00 2001 From: AmoebaProtozoa <8039876+AmoebaProtozoa@users.noreply.github.com> Date: Tue, 30 Jun 2026 10:40:33 +0800 Subject: [PATCH 15/28] external workload: thread manager explicitly --- cmd/tidb-server/main.go | 19 ++++++------ pkg/domain/BUILD.bazel | 1 + pkg/domain/domain.go | 12 ++++++++ pkg/executor/set.go | 2 +- pkg/extworkload/BUILD.bazel | 2 -- pkg/extworkload/util.go | 28 ------------------ pkg/session/bootstrap.go | 5 ++-- pkg/session/external_workload.go | 4 +-- pkg/session/session.go | 43 ++++++++++++++++++++-------- pkg/session/session_test.go | 2 +- pkg/store/driver/BUILD.bazel | 1 + pkg/store/driver/tikv_driver.go | 8 +++++- pkg/store/gcworker/gc_worker.go | 6 ++-- pkg/store/gcworker/gc_worker_test.go | 2 +- 14 files changed, 72 insertions(+), 63 deletions(-) diff --git a/cmd/tidb-server/main.go b/cmd/tidb-server/main.go index 02222ff13c264..82bc4c6632e4d 100644 --- a/cmd/tidb-server/main.go +++ b/cmd/tidb-server/main.go @@ -350,15 +350,13 @@ func initExternalWorkloadManager(ctx context.Context, storage kv.Storage) extwor logutil.BgLogger().Warn("failed to initialize external workload manager; TiDB will continue without external workload coordination", zap.Error(err)) return nil } - extworkload.SetManagerForStore(storage, mgr) return mgr } -func closeExternalWorkloadManager(storage kv.Storage, mgr extworkload.Manager) { +func closeExternalWorkloadManager(mgr extworkload.Manager) { if mgr == nil { return } - extworkload.SetManagerForStore(storage, nil) if err := mgr.Close(); err != nil { logutil.BgLogger().Warn("failed to close external workload manager", zap.Error(err)) } @@ -498,14 +496,15 @@ func main() { repository.SetupRepository(dom) if externalWorkloadManager != nil { defer func() { - closeExternalWorkloadManager(storage, externalWorkloadManager) + closeExternalWorkloadManager(externalWorkloadManager) }() } if extworkload.IsMaster(externalWorkloadManager) && pd.IsKeyspaceUsingKeyspaceLevelGC(externalWorkloadManager.Meta()) { if err = externalWorkloadManager.InitializeGCV2(context.Background()); err != nil { logutil.BgLogger().Warn("failed to initialize external workload service; TiDB will continue without external workload coordination", zap.Error(err)) - closeExternalWorkloadManager(storage, externalWorkloadManager) + closeExternalWorkloadManager(externalWorkloadManager) externalWorkloadManager = nil + dom.SetExternalWorkloadManager(nil) } } svr := createServer(storage, dom) @@ -647,13 +646,13 @@ func createStoreDDLOwnerMgrAndDomain(keyspaceName string) (kv.Storage, *domain.D if pdhttpCli != nil { pdStatus, err := pdhttpCli.GetStatus(context.Background()) if err != nil { - closeExternalWorkloadManager(storage, externalWorkloadManager) + closeExternalWorkloadManager(externalWorkloadManager) return nil, nil, nil, err } if !kerneltype.IsMatch(pdStatus.KernelType) { log.Error("kernel type mismatch", zap.String("pd", pdStatus.KernelType), zap.String("tidb", kerneltype.Name())) - closeExternalWorkloadManager(storage, externalWorkloadManager) + closeExternalWorkloadManager(externalWorkloadManager) return nil, nil, nil, errors.New("kernel type mismatch") } } @@ -663,12 +662,12 @@ func createStoreDDLOwnerMgrAndDomain(keyspaceName string) (kv.Storage, *domain.D // Bootstrap a session to load information schema. err := ddl.StartOwnerManager(context.Background(), storage) if err != nil { - closeExternalWorkloadManager(storage, externalWorkloadManager) + closeExternalWorkloadManager(externalWorkloadManager) return nil, nil, nil, err } - dom, err := session.BootstrapSession(storage) + dom, err := session.BootstrapSessionWithExternalWorkloadManager(storage, externalWorkloadManager) if err != nil { - closeExternalWorkloadManager(storage, externalWorkloadManager) + closeExternalWorkloadManager(externalWorkloadManager) return nil, nil, nil, err } return storage, dom, externalWorkloadManager, nil diff --git a/pkg/domain/BUILD.bazel b/pkg/domain/BUILD.bazel index 3b18f8d2dce0f..a5173a196349f 100644 --- a/pkg/domain/BUILD.bazel +++ b/pkg/domain/BUILD.bazel @@ -43,6 +43,7 @@ go_library( "//pkg/dxf/framework/storage", "//pkg/dxf/framework/taskexecutor", "//pkg/errno", + "//pkg/extworkload", "//pkg/infoschema", "//pkg/infoschema/issyncer", "//pkg/infoschema/isvalidator", diff --git a/pkg/domain/domain.go b/pkg/domain/domain.go index 381b8f6a4bf6e..aad3dca575dd6 100644 --- a/pkg/domain/domain.go +++ b/pkg/domain/domain.go @@ -56,6 +56,7 @@ import ( "github.com/pingcap/tidb/pkg/dxf/framework/storage" "github.com/pingcap/tidb/pkg/dxf/framework/taskexecutor" "github.com/pingcap/tidb/pkg/errno" + "github.com/pingcap/tidb/pkg/extworkload" "github.com/pingcap/tidb/pkg/infoschema" "github.com/pingcap/tidb/pkg/infoschema/issyncer" "github.com/pingcap/tidb/pkg/infoschema/isvalidator" @@ -213,6 +214,7 @@ type Domain struct { // resourceGroupsController can be changed via `SetResourceGroupsController` // in unit test. resourceGroupsController atomic.Pointer[rmclient.ResourceGroupsController] + externalWorkloadManager extworkload.Manager serverID uint64 serverIDSession *concurrency.Session @@ -1748,6 +1750,16 @@ func (do *Domain) SetResourceGroupsController(controller *rmclient.ResourceGroup do.resourceGroupsController.Store(controller) } +// ExternalWorkloadManager returns the external workload manager bound to this domain. +func (do *Domain) ExternalWorkloadManager() extworkload.Manager { + return do.externalWorkloadManager +} + +// SetExternalWorkloadManager binds the external workload manager to this domain. +func (do *Domain) SetExternalWorkloadManager(mgr extworkload.Manager) { + do.externalWorkloadManager = mgr +} + // GetRUVersion returns the current RU calculation version for this keyspace. // Returns DefaultRUVersion (v1) if the resource groups controller is not available. func (do *Domain) GetRUVersion() rmclient.RUVersion { diff --git a/pkg/executor/set.go b/pkg/executor/set.go index f5391c9644f51..3fa95d028d86e 100644 --- a/pkg/executor/set.go +++ b/pkg/executor/set.go @@ -193,7 +193,7 @@ func (e *SetExecutor) setSysVariable(ctx context.Context, name string, v *expres } logutil.BgLogger().Info(logstr, zap.Uint64("conn", sessionVars.ConnectionID), zap.String("name", name), zap.String("val", showValStr)) if v.IsGlobal && name == vardef.TiDBGCLifetime { - mgr := extworkload.GetManagerFromStore(e.Ctx().GetStore()) + mgr := domain.GetDomain(e.Ctx()).ExternalWorkloadManager() if extworkload.IsMaster(mgr) && pd.IsKeyspaceUsingKeyspaceLevelGC(mgr.Meta()) { gcLifeTime, err := time.ParseDuration(valStr) if err != nil { diff --git a/pkg/extworkload/BUILD.bazel b/pkg/extworkload/BUILD.bazel index e10f73e1bfa43..d50f2725e7d4b 100644 --- a/pkg/extworkload/BUILD.bazel +++ b/pkg/extworkload/BUILD.bazel @@ -11,9 +11,7 @@ go_library( visibility = ["//visibility:public"], deps = [ "//pkg/config", - "//pkg/config/deploymode", "//pkg/extworkload/client", - "//pkg/kv", "//pkg/metrics", "//pkg/util/logutil", "@com_github_pingcap_errors//:errors", diff --git a/pkg/extworkload/util.go b/pkg/extworkload/util.go index 83a2b78f58b27..1e53e7c9d8d71 100644 --- a/pkg/extworkload/util.go +++ b/pkg/extworkload/util.go @@ -16,8 +16,6 @@ package extworkload import ( "github.com/pingcap/tidb/pkg/config" - "github.com/pingcap/tidb/pkg/config/deploymode" - "github.com/pingcap/tidb/pkg/kv" ) // IsEnabled reports whether a Manager is present. @@ -35,32 +33,6 @@ func IsTTLTaskWorker(m Manager) bool { return roleIs(m, config.RoleTTLTaskWorker // IsAutoAnalyzeWorker reports whether this TiDB should run auto-analyze jobs. func IsAutoAnalyzeWorker(m Manager) bool { return roleIs(m, config.RoleAutoAnalyzeWorker) } -type managerStoreKey struct{} - -// SetManagerForStore binds the manager to store. Passing nil removes it. -func SetManagerForStore(store kv.Storage, mgr Manager) { - if store == nil { - return - } - store.SetOption(managerStoreKey{}, mgr) -} - -// GetManagerFromStore returns the manager bound to store only in Starter deploy mode. -func GetManagerFromStore(store kv.Storage) Manager { - if !deploymode.IsStarter() { - return nil - } - if store == nil { - return nil - } - v, ok := store.GetOption(managerStoreKey{}) - if !ok { - return nil - } - mgr, _ := v.(Manager) - return mgr -} - func roleIs(m Manager, role config.ExternalWorkloadRole) bool { return IsEnabled(m) && m.Role() == role } diff --git a/pkg/session/bootstrap.go b/pkg/session/bootstrap.go index f651978a791e1..6353bcf5863d7 100644 --- a/pkg/session/bootstrap.go +++ b/pkg/session/bootstrap.go @@ -32,6 +32,7 @@ import ( "github.com/pingcap/tidb/pkg/config" "github.com/pingcap/tidb/pkg/config/kerneltype" "github.com/pingcap/tidb/pkg/domain" + "github.com/pingcap/tidb/pkg/extworkload" "github.com/pingcap/tidb/pkg/infoschema" infoschemacontext "github.com/pingcap/tidb/pkg/infoschema/context" "github.com/pingcap/tidb/pkg/kv" @@ -60,7 +61,7 @@ import ( var bootstrapOwnerKey = "/tidb/distributeDDLOwnerLock/" // bootstrap initiates system DB for a store. -func bootstrap(s sessionapi.Session) { +func bootstrap(s sessionapi.Session, externalWorkloadManager extworkload.Manager) { startTime := time.Now() err := InitMDLVariableForBootstrap(s.GetStore()) if err != nil { @@ -76,7 +77,7 @@ func bootstrap(s sessionapi.Session) { } // For rolling upgrade, we can't do upgrade only in the owner. if b { - abortGCV2(s.GetStore()) + abortGCV2(externalWorkloadManager) upgrade(s) logutil.BgLogger().Info("upgrade successful in bootstrap", zap.Duration("take time", time.Since(startTime))) diff --git a/pkg/session/external_workload.go b/pkg/session/external_workload.go index ba216f130e27f..925a75eeda5f6 100644 --- a/pkg/session/external_workload.go +++ b/pkg/session/external_workload.go @@ -19,14 +19,12 @@ import ( "os" "github.com/pingcap/tidb/pkg/extworkload" - "github.com/pingcap/tidb/pkg/kv" "github.com/pingcap/tidb/pkg/util/intest" "github.com/pingcap/tidb/pkg/util/logutil" "go.uber.org/zap" ) -func abortGCV2(store kv.Storage) { - mgr := extworkload.GetManagerFromStore(store) +func abortGCV2(mgr extworkload.Manager) { if !extworkload.IsGCV2Worker(mgr) { return } diff --git a/pkg/session/session.go b/pkg/session/session.go index ed86c35b1b6d8..6d4a967088659 100644 --- a/pkg/session/session.go +++ b/pkg/session/session.go @@ -66,6 +66,7 @@ import ( "github.com/pingcap/tidb/pkg/expression/sessionexpr" "github.com/pingcap/tidb/pkg/extension" "github.com/pingcap/tidb/pkg/extension/extensionimpl" + "github.com/pingcap/tidb/pkg/extworkload" "github.com/pingcap/tidb/pkg/infoschema" infoschemactx "github.com/pingcap/tidb/pkg/infoschema/context" "github.com/pingcap/tidb/pkg/infoschema/issyncer" @@ -4284,12 +4285,17 @@ func InitMDLVariable(store kv.Storage) error { // BootstrapSession bootstrap session and domain. func BootstrapSession(store kv.Storage) (*domain.Domain, error) { - return bootstrapSessionImpl(context.Background(), store, createSessions) + return bootstrapSessionImpl(context.Background(), store, nil, createSessions) +} + +// BootstrapSessionWithExternalWorkloadManager bootstraps session and domain with an external workload manager. +func BootstrapSessionWithExternalWorkloadManager(store kv.Storage, externalWorkloadManager extworkload.Manager) (*domain.Domain, error) { + return bootstrapSessionImpl(context.Background(), store, externalWorkloadManager, createSessions) } // BootstrapSession4DistExecution bootstrap session and dom for Distributed execution test, only for unit testing. func BootstrapSession4DistExecution(store kv.Storage) (*domain.Domain, error) { - return bootstrapSessionImpl(context.Background(), store, createSessions4DistExecution) + return bootstrapSessionImpl(context.Background(), store, nil, createSessions4DistExecution) } // bootstrapSessionImpl bootstraps session and domain. @@ -4304,7 +4310,7 @@ func BootstrapSession4DistExecution(store kv.Storage) (*domain.Domain, error) { // - initialization global variables from system table that's required to use sessionCtx, // such as system time zone // - start domain and other routines. -func bootstrapSessionImpl(ctx context.Context, store kv.Storage, createSessionsImpl func(store kv.Storage, cnt int) ([]*session, error)) (*domain.Domain, error) { +func bootstrapSessionImpl(ctx context.Context, store kv.Storage, externalWorkloadManager extworkload.Manager, createSessionsImpl func(store kv.Storage, cnt int) ([]*session, error)) (*domain.Domain, error) { ver := getStoreBootstrapVersionWithCache(store) if kv.IsUserKS(store) { targetVer := currentBootstrapVersion @@ -4343,7 +4349,7 @@ func bootstrapSessionImpl(ctx context.Context, store kv.Storage, createSessionsI return nil, err } if ver < currentBootstrapVersion { - runInBootstrapSession(store, ver) + runInBootstrapSession(store, ver, externalWorkloadManager) } else { logutil.BgLogger().Info("cluster already bootstrapped", zap.Int64("version", ver)) err = InitMDLVariable(store) @@ -4409,6 +4415,7 @@ func bootstrapSessionImpl(ctx context.Context, store kv.Storage, createSessionsI // only start the domain after we have initialized some global variables. dom := domain.GetDomain(ses[0]) + dom.SetExternalWorkloadManager(externalWorkloadManager) err = dom.Start(ddl.Normal) if err != nil { return nil, err @@ -4544,11 +4551,8 @@ func bootstrapSessionImpl(ctx context.Context, store kv.Storage, createSessionsI dom.LoadSigningCertLoop(cfg.Security.SessionTokenSigningCert, cfg.Security.SessionTokenSigningKey) - if raw, ok := store.(kv.EtcdBackend); ok { - err = raw.StartGCWorker() - if err != nil { - return nil, err - } + if err = startGCWorker(store, externalWorkloadManager); err != nil { + return nil, err } // This only happens in testing, since the failure of loading or parsing sql file @@ -4564,6 +4568,20 @@ func bootstrapSessionImpl(ctx context.Context, store kv.Storage, createSessionsI return dom, err } +type gcWorkerStarterWithExternalWorkload interface { + StartGCWorkerWithExternalWorkload(extworkload.Manager) error +} + +func startGCWorker(store kv.Storage, externalWorkloadManager extworkload.Manager) error { + if raw, ok := store.(gcWorkerStarterWithExternalWorkload); ok { + return raw.StartGCWorkerWithExternalWorkload(externalWorkloadManager) + } + if raw, ok := store.(kv.EtcdBackend); ok { + return raw.StartGCWorker() + } + return nil +} + // GetDomain gets the associated domain for store. func GetDomain(store kv.Storage) (*domain.Domain, error) { return domap.Get(store) @@ -4588,7 +4606,7 @@ func getStartMode(ver int64) ddl.StartMode { // If no bootstrap and storage is remote, we must use a little lease time to // bootstrap quickly, after bootstrapped, we will reset the lease time. // TODO: Using a bootstrap tool for doing this may be better later. -func runInBootstrapSession(store kv.Storage, ver int64) { +func runInBootstrapSession(store kv.Storage, ver int64, externalWorkloadManager extworkload.Manager) { startMode := getStartMode(ver) startTime := time.Now() defer func() { @@ -4597,7 +4615,7 @@ func runInBootstrapSession(store kv.Storage, ver int64) { zap.Duration("cost", time.Since(startTime))) }() if startMode == ddl.Upgrade { - abortGCV2(store) + abortGCV2(externalWorkloadManager) // TODO at this time domain must not be created, else it will register server // info, and cause deadlock, we need to make sure this in a clear way logutil.BgLogger().Info("[upgrade] get owner lock to upgrade") @@ -4623,6 +4641,7 @@ func runInBootstrapSession(store kv.Storage, ver int64) { logutil.BgLogger().Fatal("createSession error", zap.Error(err)) } dom := domain.GetDomain(s) + dom.SetExternalWorkloadManager(externalWorkloadManager) err = dom.Start(startMode) if err != nil { // Bootstrap fail will cause program exit. @@ -4636,7 +4655,7 @@ func runInBootstrapSession(store kv.Storage, ver int64) { s.SetValue(sessionctx.Initing, true) switch startMode { case ddl.Bootstrap: - bootstrap(s) + bootstrap(s, externalWorkloadManager) case ddl.Upgrade: // below sleep is used to mitigate https://github.com/pingcap/tidb/issues/57003, // to let the older owner have time to notice that it's already retired. diff --git a/pkg/session/session_test.go b/pkg/session/session_test.go index f5b8d08ee8900..029bcf888e604 100644 --- a/pkg/session/session_test.go +++ b/pkg/session/session_test.go @@ -111,7 +111,7 @@ func TestBootstrapSessionImplUserKSVersionGuard(t *testing.T) { defer func() { panicVal = recover() }() - return bootstrapSessionImpl(context.Background(), userStore, createSessionStub) + return bootstrapSessionImpl(context.Background(), userStore, nil, createSessionStub) }() require.NotNil(t, panicVal) panicMsg := fmt.Sprint(panicVal) diff --git a/pkg/store/driver/BUILD.bazel b/pkg/store/driver/BUILD.bazel index 815d866b9d96a..66b5a1d81e5fe 100644 --- a/pkg/store/driver/BUILD.bazel +++ b/pkg/store/driver/BUILD.bazel @@ -6,6 +6,7 @@ go_library( importpath = "github.com/pingcap/tidb/pkg/store/driver", visibility = ["//visibility:public"], deps = [ + "//pkg/extworkload", "//pkg/kv", "//pkg/metaservice", "//pkg/metrics", diff --git a/pkg/store/driver/tikv_driver.go b/pkg/store/driver/tikv_driver.go index 4e5d4601053c9..df0522bc32b1e 100644 --- a/pkg/store/driver/tikv_driver.go +++ b/pkg/store/driver/tikv_driver.go @@ -26,6 +26,7 @@ import ( "github.com/pingcap/errors" deadlockpb "github.com/pingcap/kvproto/pkg/deadlock" "github.com/pingcap/kvproto/pkg/kvrpcpb" + "github.com/pingcap/tidb/pkg/extworkload" "github.com/pingcap/tidb/pkg/kv" "github.com/pingcap/tidb/pkg/metaservice" "github.com/pingcap/tidb/pkg/metrics" @@ -389,11 +390,16 @@ func (s *tikvStore) TLSConfig() *tls.Config { // StartGCWorker starts GC worker, it's called in BootstrapSession, don't call this function more than once. func (s *tikvStore) StartGCWorker() error { + return s.StartGCWorkerWithExternalWorkload(nil) +} + +// StartGCWorkerWithExternalWorkload starts GC worker with an external workload manager. +func (s *tikvStore) StartGCWorkerWithExternalWorkload(externalWorkloadManager extworkload.Manager) error { if !s.enableGC { return nil } - gcWorker, err := gcworker.NewGCWorker(s, s.GetPDClient()) + gcWorker, err := gcworker.NewGCWorker(s, s.GetPDClient(), externalWorkloadManager) if err != nil { return derr.ToTiDBErr(err) } diff --git a/pkg/store/gcworker/gc_worker.go b/pkg/store/gcworker/gc_worker.go index 7ace9f2f4a3a2..018e7d8226a87 100644 --- a/pkg/store/gcworker/gc_worker.go +++ b/pkg/store/gcworker/gc_worker.go @@ -77,6 +77,7 @@ type GCWorker struct { tikvStore tikv.Storage pdClient pd.Client pdGCControllerClient pdgc.InternalController + externalWorkload extworkload.Manager gcIsRunning bool lastFinish time.Time cancel context.CancelFunc @@ -90,7 +91,7 @@ type GCWorker struct { } // NewGCWorker creates a GCWorker instance. -func NewGCWorker(store kv.Storage, pdClient pd.Client) (*GCWorker, error) { +func NewGCWorker(store kv.Storage, pdClient pd.Client, externalWorkloadManager extworkload.Manager) (*GCWorker, error) { ver, err := store.CurrentVersion(kv.GlobalTxnScope) if err != nil { return nil, errors.Trace(err) @@ -115,6 +116,7 @@ func NewGCWorker(store kv.Storage, pdClient pd.Client) (*GCWorker, error) { tikvStore: tikvStore, pdClient: gcPDClient, pdGCControllerClient: gcPDClient.GetGCInternalController(keyspaceID), + externalWorkload: externalWorkloadManager, gcIsRunning: false, lastFinish: time.Now(), regionLockResolver: tikv.NewRegionLockResolver(resolverIdentifier, tikvStore), @@ -849,7 +851,7 @@ func (w *GCWorker) runGCJob(ctx context.Context, safePoint uint64, concurrency g } func (w *GCWorker) notifyGCV2AfterGC(ctx context.Context, safePoint uint64) error { - mgr := extworkload.GetManagerFromStore(w.store) + mgr := w.externalWorkload if !extworkload.IsEnabled(mgr) || !pd.IsKeyspaceUsingKeyspaceLevelGC(mgr.Meta()) { return nil } diff --git a/pkg/store/gcworker/gc_worker_test.go b/pkg/store/gcworker/gc_worker_test.go index cda4d854ab371..6cd83d4faa1a3 100644 --- a/pkg/store/gcworker/gc_worker_test.go +++ b/pkg/store/gcworker/gc_worker_test.go @@ -215,7 +215,7 @@ func createGCWorkerSuite(t *testing.T, opts ...mockGCWorkerSuiteOption) *mockGCW s.tikvStore = s.store.(tikv.Storage) - gcWorker, err := NewGCWorker(s.store, s.pdClient) + gcWorker, err := NewGCWorker(s.store, s.pdClient, nil) require.NoError(t, err) gcWorker.Start() gcWorker.Close() From 23f740c29c457fa3d0bd36ca5f09e41bd4b32bf0 Mon Sep 17 00:00:00 2001 From: AmoebaProtozoa <8039876+AmoebaProtozoa@users.noreply.github.com> Date: Tue, 30 Jun 2026 10:45:01 +0800 Subject: [PATCH 16/28] external workload: delay manager init until storage validated --- cmd/tidb-server/main.go | 4 +--- pkg/extworkload/util.go | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/cmd/tidb-server/main.go b/cmd/tidb-server/main.go index 82bc4c6632e4d..d95f9893c0e26 100644 --- a/cmd/tidb-server/main.go +++ b/cmd/tidb-server/main.go @@ -639,24 +639,22 @@ func createStoreDDLOwnerMgrAndDomain(keyspaceName string) (kv.Storage, *domain.D kv.StandAloneTiDB = true } storage := kvstore.MustInitStorage(keyspaceName) - externalWorkloadManager := initExternalWorkloadManager(context.Background(), storage) if tikvStore, ok := storage.(kv.StorageWithPD); ok { pdhttpCli := tikvStore.GetPDHTTPClient() // unistore also implements kv.StorageWithPD, but it does not have PD client. if pdhttpCli != nil { pdStatus, err := pdhttpCli.GetStatus(context.Background()) if err != nil { - closeExternalWorkloadManager(externalWorkloadManager) return nil, nil, nil, err } if !kerneltype.IsMatch(pdStatus.KernelType) { log.Error("kernel type mismatch", zap.String("pd", pdStatus.KernelType), zap.String("tidb", kerneltype.Name())) - closeExternalWorkloadManager(externalWorkloadManager) return nil, nil, nil, errors.New("kernel type mismatch") } } } + externalWorkloadManager := initExternalWorkloadManager(context.Background(), storage) copr.GlobalMPPFailedStoreProber.Run() mppcoordmanager.InstanceMPPCoordinatorManager.Run() // Bootstrap a session to load information schema. diff --git a/pkg/extworkload/util.go b/pkg/extworkload/util.go index 1e53e7c9d8d71..db17bb6a4a82f 100644 --- a/pkg/extworkload/util.go +++ b/pkg/extworkload/util.go @@ -14,9 +14,7 @@ package extworkload -import ( - "github.com/pingcap/tidb/pkg/config" -) +import "github.com/pingcap/tidb/pkg/config" // IsEnabled reports whether a Manager is present. func IsEnabled(m Manager) bool { return m != nil } From 8fc1a4cead1e3dd9e7ef8b0ec758e533616217bf Mon Sep 17 00:00:00 2001 From: AmoebaProtozoa <8039876+AmoebaProtozoa@users.noreply.github.com> Date: Tue, 30 Jun 2026 10:48:38 +0800 Subject: [PATCH 17/28] external workload: fix GCV2 fallback ordering --- cmd/tidb-server/main.go | 15 +++++++-------- pkg/executor/set.go | 5 ++++- pkg/session/bootstrap.go | 4 +--- pkg/session/session.go | 6 ++++-- 4 files changed, 16 insertions(+), 14 deletions(-) diff --git a/cmd/tidb-server/main.go b/cmd/tidb-server/main.go index d95f9893c0e26..3e3ddeaffc59e 100644 --- a/cmd/tidb-server/main.go +++ b/cmd/tidb-server/main.go @@ -499,14 +499,6 @@ func main() { closeExternalWorkloadManager(externalWorkloadManager) }() } - if extworkload.IsMaster(externalWorkloadManager) && pd.IsKeyspaceUsingKeyspaceLevelGC(externalWorkloadManager.Meta()) { - if err = externalWorkloadManager.InitializeGCV2(context.Background()); err != nil { - logutil.BgLogger().Warn("failed to initialize external workload service; TiDB will continue without external workload coordination", zap.Error(err)) - closeExternalWorkloadManager(externalWorkloadManager) - externalWorkloadManager = nil - dom.SetExternalWorkloadManager(nil) - } - } svr := createServer(storage, dom) if standbyController != nil { svr.StandbyController = standbyController @@ -655,6 +647,13 @@ func createStoreDDLOwnerMgrAndDomain(keyspaceName string) (kv.Storage, *domain.D } } externalWorkloadManager := initExternalWorkloadManager(context.Background(), storage) + if extworkload.IsMaster(externalWorkloadManager) && pd.IsKeyspaceUsingKeyspaceLevelGC(externalWorkloadManager.Meta()) { + if err := externalWorkloadManager.InitializeGCV2(context.Background()); err != nil { + logutil.BgLogger().Warn("failed to initialize external workload service; TiDB will continue without external workload coordination", zap.Error(err)) + closeExternalWorkloadManager(externalWorkloadManager) + externalWorkloadManager = nil + } + } copr.GlobalMPPFailedStoreProber.Run() mppcoordmanager.InstanceMPPCoordinatorManager.Run() // Bootstrap a session to load information schema. diff --git a/pkg/executor/set.go b/pkg/executor/set.go index 3fa95d028d86e..110209e5cf4ea 100644 --- a/pkg/executor/set.go +++ b/pkg/executor/set.go @@ -200,7 +200,10 @@ func (e *SetExecutor) setSysVariable(ctx context.Context, name string, v *expres return err } if err = mgr.UpdateGCLifeTime(ctx, int64(gcLifeTime/time.Second)); err != nil { - return err + logutil.BgLogger().Warn("failed to update external workload GC life time", + zap.String("name", name), + zap.String("val", showValStr), + zap.Error(err)) } } } diff --git a/pkg/session/bootstrap.go b/pkg/session/bootstrap.go index 6353bcf5863d7..a95cf2fee9693 100644 --- a/pkg/session/bootstrap.go +++ b/pkg/session/bootstrap.go @@ -32,7 +32,6 @@ import ( "github.com/pingcap/tidb/pkg/config" "github.com/pingcap/tidb/pkg/config/kerneltype" "github.com/pingcap/tidb/pkg/domain" - "github.com/pingcap/tidb/pkg/extworkload" "github.com/pingcap/tidb/pkg/infoschema" infoschemacontext "github.com/pingcap/tidb/pkg/infoschema/context" "github.com/pingcap/tidb/pkg/kv" @@ -61,7 +60,7 @@ import ( var bootstrapOwnerKey = "/tidb/distributeDDLOwnerLock/" // bootstrap initiates system DB for a store. -func bootstrap(s sessionapi.Session, externalWorkloadManager extworkload.Manager) { +func bootstrap(s sessionapi.Session) { startTime := time.Now() err := InitMDLVariableForBootstrap(s.GetStore()) if err != nil { @@ -77,7 +76,6 @@ func bootstrap(s sessionapi.Session, externalWorkloadManager extworkload.Manager } // For rolling upgrade, we can't do upgrade only in the owner. if b { - abortGCV2(externalWorkloadManager) upgrade(s) logutil.BgLogger().Info("upgrade successful in bootstrap", zap.Duration("take time", time.Since(startTime))) diff --git a/pkg/session/session.go b/pkg/session/session.go index 6d4a967088659..7777cc9ae716e 100644 --- a/pkg/session/session.go +++ b/pkg/session/session.go @@ -4614,8 +4614,10 @@ func runInBootstrapSession(store kv.Storage, ver int64, externalWorkloadManager zap.String("bootMode", string(startMode)), zap.Duration("cost", time.Since(startTime))) }() - if startMode == ddl.Upgrade { + if startMode != ddl.Normal { abortGCV2(externalWorkloadManager) + } + if startMode == ddl.Upgrade { // TODO at this time domain must not be created, else it will register server // info, and cause deadlock, we need to make sure this in a clear way logutil.BgLogger().Info("[upgrade] get owner lock to upgrade") @@ -4655,7 +4657,7 @@ func runInBootstrapSession(store kv.Storage, ver int64, externalWorkloadManager s.SetValue(sessionctx.Initing, true) switch startMode { case ddl.Bootstrap: - bootstrap(s, externalWorkloadManager) + bootstrap(s) case ddl.Upgrade: // below sleep is used to mitigate https://github.com/pingcap/tidb/issues/57003, // to let the older owner have time to notice that it's already retired. From 859345232d7ee1e7376917201a5692b4108e1836 Mon Sep 17 00:00:00 2001 From: AmoebaProtozoa <8039876+AmoebaProtozoa@users.noreply.github.com> Date: Tue, 30 Jun 2026 11:03:44 +0800 Subject: [PATCH 18/28] external workload: keep manager on storage --- cmd/tidb-server/main.go | 30 ++++++++++---------- pkg/domain/BUILD.bazel | 1 - pkg/domain/domain.go | 12 -------- pkg/executor/set.go | 2 +- pkg/extworkload/BUILD.bazel | 2 ++ pkg/extworkload/util.go | 29 +++++++++++++++++++- pkg/session/external_workload.go | 4 ++- pkg/session/session.go | 41 ++++++++-------------------- pkg/session/session_test.go | 2 +- pkg/store/driver/BUILD.bazel | 1 - pkg/store/driver/tikv_driver.go | 8 +----- pkg/store/gcworker/gc_worker.go | 6 ++-- pkg/store/gcworker/gc_worker_test.go | 2 +- 13 files changed, 66 insertions(+), 74 deletions(-) diff --git a/cmd/tidb-server/main.go b/cmd/tidb-server/main.go index 3e3ddeaffc59e..9f33ea52389a2 100644 --- a/cmd/tidb-server/main.go +++ b/cmd/tidb-server/main.go @@ -350,13 +350,15 @@ func initExternalWorkloadManager(ctx context.Context, storage kv.Storage) extwor logutil.BgLogger().Warn("failed to initialize external workload manager; TiDB will continue without external workload coordination", zap.Error(err)) return nil } + extworkload.SetManagerForStore(storage, mgr) return mgr } -func closeExternalWorkloadManager(mgr extworkload.Manager) { +func closeExternalWorkloadManager(storage kv.Storage, mgr extworkload.Manager) { if mgr == nil { return } + extworkload.SetManagerForStore(storage, nil) if err := mgr.Close(); err != nil { logutil.BgLogger().Warn("failed to close external workload manager", zap.Error(err)) } @@ -491,12 +493,12 @@ func main() { keyspaceName := keyspace.GetKeyspaceNameBySettings() executor.Start() resourcemanager.InstanceResourceManager.Start() - storage, dom, externalWorkloadManager, err := createStoreDDLOwnerMgrAndDomain(keyspaceName) + storage, dom, err := createStoreDDLOwnerMgrAndDomain(keyspaceName) terror.MustNil(err) repository.SetupRepository(dom) - if externalWorkloadManager != nil { + if externalWorkloadManager := extworkload.GetManagerFromStore(storage); externalWorkloadManager != nil { defer func() { - closeExternalWorkloadManager(externalWorkloadManager) + closeExternalWorkloadManager(storage, externalWorkloadManager) }() } svr := createServer(storage, dom) @@ -626,7 +628,7 @@ func registerStores() error { return err } -func createStoreDDLOwnerMgrAndDomain(keyspaceName string) (kv.Storage, *domain.Domain, extworkload.Manager, error) { +func createStoreDDLOwnerMgrAndDomain(keyspaceName string) (kv.Storage, *domain.Domain, error) { if config.GetGlobalConfig().Store == config.StoreTypeUniStore { kv.StandAloneTiDB = true } @@ -637,12 +639,12 @@ func createStoreDDLOwnerMgrAndDomain(keyspaceName string) (kv.Storage, *domain.D if pdhttpCli != nil { pdStatus, err := pdhttpCli.GetStatus(context.Background()) if err != nil { - return nil, nil, nil, err + return nil, nil, err } if !kerneltype.IsMatch(pdStatus.KernelType) { log.Error("kernel type mismatch", zap.String("pd", pdStatus.KernelType), zap.String("tidb", kerneltype.Name())) - return nil, nil, nil, errors.New("kernel type mismatch") + return nil, nil, errors.New("kernel type mismatch") } } } @@ -650,7 +652,7 @@ func createStoreDDLOwnerMgrAndDomain(keyspaceName string) (kv.Storage, *domain.D if extworkload.IsMaster(externalWorkloadManager) && pd.IsKeyspaceUsingKeyspaceLevelGC(externalWorkloadManager.Meta()) { if err := externalWorkloadManager.InitializeGCV2(context.Background()); err != nil { logutil.BgLogger().Warn("failed to initialize external workload service; TiDB will continue without external workload coordination", zap.Error(err)) - closeExternalWorkloadManager(externalWorkloadManager) + closeExternalWorkloadManager(storage, externalWorkloadManager) externalWorkloadManager = nil } } @@ -659,15 +661,15 @@ func createStoreDDLOwnerMgrAndDomain(keyspaceName string) (kv.Storage, *domain.D // Bootstrap a session to load information schema. err := ddl.StartOwnerManager(context.Background(), storage) if err != nil { - closeExternalWorkloadManager(externalWorkloadManager) - return nil, nil, nil, err + closeExternalWorkloadManager(storage, externalWorkloadManager) + return nil, nil, err } - dom, err := session.BootstrapSessionWithExternalWorkloadManager(storage, externalWorkloadManager) + dom, err := session.BootstrapSession(storage) if err != nil { - closeExternalWorkloadManager(externalWorkloadManager) - return nil, nil, nil, err + closeExternalWorkloadManager(storage, externalWorkloadManager) + return nil, nil, err } - return storage, dom, externalWorkloadManager, nil + return storage, dom, nil } // Prometheus push. diff --git a/pkg/domain/BUILD.bazel b/pkg/domain/BUILD.bazel index a5173a196349f..3b18f8d2dce0f 100644 --- a/pkg/domain/BUILD.bazel +++ b/pkg/domain/BUILD.bazel @@ -43,7 +43,6 @@ go_library( "//pkg/dxf/framework/storage", "//pkg/dxf/framework/taskexecutor", "//pkg/errno", - "//pkg/extworkload", "//pkg/infoschema", "//pkg/infoschema/issyncer", "//pkg/infoschema/isvalidator", diff --git a/pkg/domain/domain.go b/pkg/domain/domain.go index aad3dca575dd6..381b8f6a4bf6e 100644 --- a/pkg/domain/domain.go +++ b/pkg/domain/domain.go @@ -56,7 +56,6 @@ import ( "github.com/pingcap/tidb/pkg/dxf/framework/storage" "github.com/pingcap/tidb/pkg/dxf/framework/taskexecutor" "github.com/pingcap/tidb/pkg/errno" - "github.com/pingcap/tidb/pkg/extworkload" "github.com/pingcap/tidb/pkg/infoschema" "github.com/pingcap/tidb/pkg/infoschema/issyncer" "github.com/pingcap/tidb/pkg/infoschema/isvalidator" @@ -214,7 +213,6 @@ type Domain struct { // resourceGroupsController can be changed via `SetResourceGroupsController` // in unit test. resourceGroupsController atomic.Pointer[rmclient.ResourceGroupsController] - externalWorkloadManager extworkload.Manager serverID uint64 serverIDSession *concurrency.Session @@ -1750,16 +1748,6 @@ func (do *Domain) SetResourceGroupsController(controller *rmclient.ResourceGroup do.resourceGroupsController.Store(controller) } -// ExternalWorkloadManager returns the external workload manager bound to this domain. -func (do *Domain) ExternalWorkloadManager() extworkload.Manager { - return do.externalWorkloadManager -} - -// SetExternalWorkloadManager binds the external workload manager to this domain. -func (do *Domain) SetExternalWorkloadManager(mgr extworkload.Manager) { - do.externalWorkloadManager = mgr -} - // GetRUVersion returns the current RU calculation version for this keyspace. // Returns DefaultRUVersion (v1) if the resource groups controller is not available. func (do *Domain) GetRUVersion() rmclient.RUVersion { diff --git a/pkg/executor/set.go b/pkg/executor/set.go index 110209e5cf4ea..92dfae7b50716 100644 --- a/pkg/executor/set.go +++ b/pkg/executor/set.go @@ -193,7 +193,7 @@ func (e *SetExecutor) setSysVariable(ctx context.Context, name string, v *expres } logutil.BgLogger().Info(logstr, zap.Uint64("conn", sessionVars.ConnectionID), zap.String("name", name), zap.String("val", showValStr)) if v.IsGlobal && name == vardef.TiDBGCLifetime { - mgr := domain.GetDomain(e.Ctx()).ExternalWorkloadManager() + mgr := extworkload.GetManagerFromStore(e.Ctx().GetStore()) if extworkload.IsMaster(mgr) && pd.IsKeyspaceUsingKeyspaceLevelGC(mgr.Meta()) { gcLifeTime, err := time.ParseDuration(valStr) if err != nil { diff --git a/pkg/extworkload/BUILD.bazel b/pkg/extworkload/BUILD.bazel index d50f2725e7d4b..e10f73e1bfa43 100644 --- a/pkg/extworkload/BUILD.bazel +++ b/pkg/extworkload/BUILD.bazel @@ -11,7 +11,9 @@ go_library( visibility = ["//visibility:public"], deps = [ "//pkg/config", + "//pkg/config/deploymode", "//pkg/extworkload/client", + "//pkg/kv", "//pkg/metrics", "//pkg/util/logutil", "@com_github_pingcap_errors//:errors", diff --git a/pkg/extworkload/util.go b/pkg/extworkload/util.go index db17bb6a4a82f..32df67eb3efe0 100644 --- a/pkg/extworkload/util.go +++ b/pkg/extworkload/util.go @@ -14,7 +14,11 @@ package extworkload -import "github.com/pingcap/tidb/pkg/config" +import ( + "github.com/pingcap/tidb/pkg/config" + "github.com/pingcap/tidb/pkg/config/deploymode" + "github.com/pingcap/tidb/pkg/kv" +) // IsEnabled reports whether a Manager is present. func IsEnabled(m Manager) bool { return m != nil } @@ -31,6 +35,29 @@ func IsTTLTaskWorker(m Manager) bool { return roleIs(m, config.RoleTTLTaskWorker // IsAutoAnalyzeWorker reports whether this TiDB should run auto-analyze jobs. func IsAutoAnalyzeWorker(m Manager) bool { return roleIs(m, config.RoleAutoAnalyzeWorker) } +type managerStoreKey struct{} + +// SetManagerForStore binds the manager to store. Passing nil removes it. +func SetManagerForStore(store kv.Storage, mgr Manager) { + if store == nil { + return + } + store.SetOption(managerStoreKey{}, mgr) +} + +// GetManagerFromStore returns the manager bound to store only in Starter deploy mode. +func GetManagerFromStore(store kv.Storage) Manager { + if !deploymode.IsStarter() || store == nil { + return nil + } + v, ok := store.GetOption(managerStoreKey{}) + if !ok { + return nil + } + mgr, _ := v.(Manager) + return mgr +} + func roleIs(m Manager, role config.ExternalWorkloadRole) bool { return IsEnabled(m) && m.Role() == role } diff --git a/pkg/session/external_workload.go b/pkg/session/external_workload.go index 925a75eeda5f6..ba216f130e27f 100644 --- a/pkg/session/external_workload.go +++ b/pkg/session/external_workload.go @@ -19,12 +19,14 @@ import ( "os" "github.com/pingcap/tidb/pkg/extworkload" + "github.com/pingcap/tidb/pkg/kv" "github.com/pingcap/tidb/pkg/util/intest" "github.com/pingcap/tidb/pkg/util/logutil" "go.uber.org/zap" ) -func abortGCV2(mgr extworkload.Manager) { +func abortGCV2(store kv.Storage) { + mgr := extworkload.GetManagerFromStore(store) if !extworkload.IsGCV2Worker(mgr) { return } diff --git a/pkg/session/session.go b/pkg/session/session.go index 7777cc9ae716e..6468e7a97a27d 100644 --- a/pkg/session/session.go +++ b/pkg/session/session.go @@ -66,7 +66,6 @@ import ( "github.com/pingcap/tidb/pkg/expression/sessionexpr" "github.com/pingcap/tidb/pkg/extension" "github.com/pingcap/tidb/pkg/extension/extensionimpl" - "github.com/pingcap/tidb/pkg/extworkload" "github.com/pingcap/tidb/pkg/infoschema" infoschemactx "github.com/pingcap/tidb/pkg/infoschema/context" "github.com/pingcap/tidb/pkg/infoschema/issyncer" @@ -4285,17 +4284,12 @@ func InitMDLVariable(store kv.Storage) error { // BootstrapSession bootstrap session and domain. func BootstrapSession(store kv.Storage) (*domain.Domain, error) { - return bootstrapSessionImpl(context.Background(), store, nil, createSessions) -} - -// BootstrapSessionWithExternalWorkloadManager bootstraps session and domain with an external workload manager. -func BootstrapSessionWithExternalWorkloadManager(store kv.Storage, externalWorkloadManager extworkload.Manager) (*domain.Domain, error) { - return bootstrapSessionImpl(context.Background(), store, externalWorkloadManager, createSessions) + return bootstrapSessionImpl(context.Background(), store, createSessions) } // BootstrapSession4DistExecution bootstrap session and dom for Distributed execution test, only for unit testing. func BootstrapSession4DistExecution(store kv.Storage) (*domain.Domain, error) { - return bootstrapSessionImpl(context.Background(), store, nil, createSessions4DistExecution) + return bootstrapSessionImpl(context.Background(), store, createSessions4DistExecution) } // bootstrapSessionImpl bootstraps session and domain. @@ -4310,7 +4304,7 @@ func BootstrapSession4DistExecution(store kv.Storage) (*domain.Domain, error) { // - initialization global variables from system table that's required to use sessionCtx, // such as system time zone // - start domain and other routines. -func bootstrapSessionImpl(ctx context.Context, store kv.Storage, externalWorkloadManager extworkload.Manager, createSessionsImpl func(store kv.Storage, cnt int) ([]*session, error)) (*domain.Domain, error) { +func bootstrapSessionImpl(ctx context.Context, store kv.Storage, createSessionsImpl func(store kv.Storage, cnt int) ([]*session, error)) (*domain.Domain, error) { ver := getStoreBootstrapVersionWithCache(store) if kv.IsUserKS(store) { targetVer := currentBootstrapVersion @@ -4349,7 +4343,7 @@ func bootstrapSessionImpl(ctx context.Context, store kv.Storage, externalWorkloa return nil, err } if ver < currentBootstrapVersion { - runInBootstrapSession(store, ver, externalWorkloadManager) + runInBootstrapSession(store, ver) } else { logutil.BgLogger().Info("cluster already bootstrapped", zap.Int64("version", ver)) err = InitMDLVariable(store) @@ -4415,7 +4409,6 @@ func bootstrapSessionImpl(ctx context.Context, store kv.Storage, externalWorkloa // only start the domain after we have initialized some global variables. dom := domain.GetDomain(ses[0]) - dom.SetExternalWorkloadManager(externalWorkloadManager) err = dom.Start(ddl.Normal) if err != nil { return nil, err @@ -4551,8 +4544,11 @@ func bootstrapSessionImpl(ctx context.Context, store kv.Storage, externalWorkloa dom.LoadSigningCertLoop(cfg.Security.SessionTokenSigningCert, cfg.Security.SessionTokenSigningKey) - if err = startGCWorker(store, externalWorkloadManager); err != nil { - return nil, err + if raw, ok := store.(kv.EtcdBackend); ok { + err = raw.StartGCWorker() + if err != nil { + return nil, err + } } // This only happens in testing, since the failure of loading or parsing sql file @@ -4568,20 +4564,6 @@ func bootstrapSessionImpl(ctx context.Context, store kv.Storage, externalWorkloa return dom, err } -type gcWorkerStarterWithExternalWorkload interface { - StartGCWorkerWithExternalWorkload(extworkload.Manager) error -} - -func startGCWorker(store kv.Storage, externalWorkloadManager extworkload.Manager) error { - if raw, ok := store.(gcWorkerStarterWithExternalWorkload); ok { - return raw.StartGCWorkerWithExternalWorkload(externalWorkloadManager) - } - if raw, ok := store.(kv.EtcdBackend); ok { - return raw.StartGCWorker() - } - return nil -} - // GetDomain gets the associated domain for store. func GetDomain(store kv.Storage) (*domain.Domain, error) { return domap.Get(store) @@ -4606,7 +4588,7 @@ func getStartMode(ver int64) ddl.StartMode { // If no bootstrap and storage is remote, we must use a little lease time to // bootstrap quickly, after bootstrapped, we will reset the lease time. // TODO: Using a bootstrap tool for doing this may be better later. -func runInBootstrapSession(store kv.Storage, ver int64, externalWorkloadManager extworkload.Manager) { +func runInBootstrapSession(store kv.Storage, ver int64) { startMode := getStartMode(ver) startTime := time.Now() defer func() { @@ -4615,7 +4597,7 @@ func runInBootstrapSession(store kv.Storage, ver int64, externalWorkloadManager zap.Duration("cost", time.Since(startTime))) }() if startMode != ddl.Normal { - abortGCV2(externalWorkloadManager) + abortGCV2(store) } if startMode == ddl.Upgrade { // TODO at this time domain must not be created, else it will register server @@ -4643,7 +4625,6 @@ func runInBootstrapSession(store kv.Storage, ver int64, externalWorkloadManager logutil.BgLogger().Fatal("createSession error", zap.Error(err)) } dom := domain.GetDomain(s) - dom.SetExternalWorkloadManager(externalWorkloadManager) err = dom.Start(startMode) if err != nil { // Bootstrap fail will cause program exit. diff --git a/pkg/session/session_test.go b/pkg/session/session_test.go index 029bcf888e604..f5b8d08ee8900 100644 --- a/pkg/session/session_test.go +++ b/pkg/session/session_test.go @@ -111,7 +111,7 @@ func TestBootstrapSessionImplUserKSVersionGuard(t *testing.T) { defer func() { panicVal = recover() }() - return bootstrapSessionImpl(context.Background(), userStore, nil, createSessionStub) + return bootstrapSessionImpl(context.Background(), userStore, createSessionStub) }() require.NotNil(t, panicVal) panicMsg := fmt.Sprint(panicVal) diff --git a/pkg/store/driver/BUILD.bazel b/pkg/store/driver/BUILD.bazel index 66b5a1d81e5fe..815d866b9d96a 100644 --- a/pkg/store/driver/BUILD.bazel +++ b/pkg/store/driver/BUILD.bazel @@ -6,7 +6,6 @@ go_library( importpath = "github.com/pingcap/tidb/pkg/store/driver", visibility = ["//visibility:public"], deps = [ - "//pkg/extworkload", "//pkg/kv", "//pkg/metaservice", "//pkg/metrics", diff --git a/pkg/store/driver/tikv_driver.go b/pkg/store/driver/tikv_driver.go index df0522bc32b1e..4e5d4601053c9 100644 --- a/pkg/store/driver/tikv_driver.go +++ b/pkg/store/driver/tikv_driver.go @@ -26,7 +26,6 @@ import ( "github.com/pingcap/errors" deadlockpb "github.com/pingcap/kvproto/pkg/deadlock" "github.com/pingcap/kvproto/pkg/kvrpcpb" - "github.com/pingcap/tidb/pkg/extworkload" "github.com/pingcap/tidb/pkg/kv" "github.com/pingcap/tidb/pkg/metaservice" "github.com/pingcap/tidb/pkg/metrics" @@ -390,16 +389,11 @@ func (s *tikvStore) TLSConfig() *tls.Config { // StartGCWorker starts GC worker, it's called in BootstrapSession, don't call this function more than once. func (s *tikvStore) StartGCWorker() error { - return s.StartGCWorkerWithExternalWorkload(nil) -} - -// StartGCWorkerWithExternalWorkload starts GC worker with an external workload manager. -func (s *tikvStore) StartGCWorkerWithExternalWorkload(externalWorkloadManager extworkload.Manager) error { if !s.enableGC { return nil } - gcWorker, err := gcworker.NewGCWorker(s, s.GetPDClient(), externalWorkloadManager) + gcWorker, err := gcworker.NewGCWorker(s, s.GetPDClient()) if err != nil { return derr.ToTiDBErr(err) } diff --git a/pkg/store/gcworker/gc_worker.go b/pkg/store/gcworker/gc_worker.go index 018e7d8226a87..7ace9f2f4a3a2 100644 --- a/pkg/store/gcworker/gc_worker.go +++ b/pkg/store/gcworker/gc_worker.go @@ -77,7 +77,6 @@ type GCWorker struct { tikvStore tikv.Storage pdClient pd.Client pdGCControllerClient pdgc.InternalController - externalWorkload extworkload.Manager gcIsRunning bool lastFinish time.Time cancel context.CancelFunc @@ -91,7 +90,7 @@ type GCWorker struct { } // NewGCWorker creates a GCWorker instance. -func NewGCWorker(store kv.Storage, pdClient pd.Client, externalWorkloadManager extworkload.Manager) (*GCWorker, error) { +func NewGCWorker(store kv.Storage, pdClient pd.Client) (*GCWorker, error) { ver, err := store.CurrentVersion(kv.GlobalTxnScope) if err != nil { return nil, errors.Trace(err) @@ -116,7 +115,6 @@ func NewGCWorker(store kv.Storage, pdClient pd.Client, externalWorkloadManager e tikvStore: tikvStore, pdClient: gcPDClient, pdGCControllerClient: gcPDClient.GetGCInternalController(keyspaceID), - externalWorkload: externalWorkloadManager, gcIsRunning: false, lastFinish: time.Now(), regionLockResolver: tikv.NewRegionLockResolver(resolverIdentifier, tikvStore), @@ -851,7 +849,7 @@ func (w *GCWorker) runGCJob(ctx context.Context, safePoint uint64, concurrency g } func (w *GCWorker) notifyGCV2AfterGC(ctx context.Context, safePoint uint64) error { - mgr := w.externalWorkload + mgr := extworkload.GetManagerFromStore(w.store) if !extworkload.IsEnabled(mgr) || !pd.IsKeyspaceUsingKeyspaceLevelGC(mgr.Meta()) { return nil } diff --git a/pkg/store/gcworker/gc_worker_test.go b/pkg/store/gcworker/gc_worker_test.go index 6cd83d4faa1a3..cda4d854ab371 100644 --- a/pkg/store/gcworker/gc_worker_test.go +++ b/pkg/store/gcworker/gc_worker_test.go @@ -215,7 +215,7 @@ func createGCWorkerSuite(t *testing.T, opts ...mockGCWorkerSuiteOption) *mockGCW s.tikvStore = s.store.(tikv.Storage) - gcWorker, err := NewGCWorker(s.store, s.pdClient, nil) + gcWorker, err := NewGCWorker(s.store, s.pdClient) require.NoError(t, err) gcWorker.Start() gcWorker.Close() From 2babebcc54d1580edd56ef99b0710b9b2a26b7c6 Mon Sep 17 00:00:00 2001 From: AmoebaProtozoa <8039876+AmoebaProtozoa@users.noreply.github.com> Date: Tue, 30 Jun 2026 11:17:06 +0800 Subject: [PATCH 19/28] external workload: simplify GCV2 notification --- pkg/extworkload/BUILD.bazel | 1 - pkg/extworkload/util.go | 5 ++- pkg/store/gcworker/gc_worker.go | 46 ++++++++-------------------- pkg/store/gcworker/gc_worker_test.go | 39 ----------------------- 4 files changed, 15 insertions(+), 76 deletions(-) diff --git a/pkg/extworkload/BUILD.bazel b/pkg/extworkload/BUILD.bazel index e10f73e1bfa43..ae7590a1d0304 100644 --- a/pkg/extworkload/BUILD.bazel +++ b/pkg/extworkload/BUILD.bazel @@ -11,7 +11,6 @@ go_library( visibility = ["//visibility:public"], deps = [ "//pkg/config", - "//pkg/config/deploymode", "//pkg/extworkload/client", "//pkg/kv", "//pkg/metrics", diff --git a/pkg/extworkload/util.go b/pkg/extworkload/util.go index 32df67eb3efe0..47e155a50f0a3 100644 --- a/pkg/extworkload/util.go +++ b/pkg/extworkload/util.go @@ -16,7 +16,6 @@ package extworkload import ( "github.com/pingcap/tidb/pkg/config" - "github.com/pingcap/tidb/pkg/config/deploymode" "github.com/pingcap/tidb/pkg/kv" ) @@ -45,9 +44,9 @@ func SetManagerForStore(store kv.Storage, mgr Manager) { store.SetOption(managerStoreKey{}, mgr) } -// GetManagerFromStore returns the manager bound to store only in Starter deploy mode. +// GetManagerFromStore returns the manager bound to store. func GetManagerFromStore(store kv.Storage) Manager { - if !deploymode.IsStarter() || store == nil { + if store == nil { return nil } v, ok := store.GetOption(managerStoreKey{}) diff --git a/pkg/store/gcworker/gc_worker.go b/pkg/store/gcworker/gc_worker.go index 7ace9f2f4a3a2..b31a3a23cddbc 100644 --- a/pkg/store/gcworker/gc_worker.go +++ b/pkg/store/gcworker/gc_worker.go @@ -854,13 +854,8 @@ func (w *GCWorker) notifyGCV2AfterGC(ctx context.Context, safePoint uint64) erro return nil } - shouldRegister, shouldRecycle := gcv2NotificationActions(mgr.Role()) - if !shouldRegister && !shouldRecycle { - return nil - } - - var gcLifeTimeSec int64 - if shouldRegister { + switch mgr.Role() { + case config.RoleMaster, config.RoleTTLTaskWorker: gcLifeTime, err := w.loadDurationWithDefault(gcLifeTimeKey, gcDefaultLifeTime) if err != nil { logutil.Logger(ctx).Error("failed to load GC life time for external workload", @@ -868,41 +863,26 @@ func (w *GCWorker) notifyGCV2AfterGC(ctx context.Context, safePoint uint64) erro zap.Error(err)) return errors.Trace(err) } - gcLifeTimeSec = int64(*gcLifeTime / time.Second) - } - - if shouldRegister { - err := mgr.RegisterGCV2(ctx, safePoint, gcLifeTimeSec) - if err != nil { + if err := mgr.RegisterGCV2(ctx, safePoint, int64(*gcLifeTime/time.Second)); err != nil { logutil.Logger(ctx).Error("failed to register GCV2 task", zap.String("category", "gc worker"), zap.Uint64("safePoint", safePoint), zap.Error(err)) return errors.Trace(err) } - } - if shouldRecycle { - err := mgr.RecycleGCV2(ctx, safePoint) - if err != nil { - logutil.Logger(ctx).Error("failed to recycle GCV2 task", - zap.String("category", "gc worker"), - zap.Uint64("safePoint", safePoint), - zap.Error(err)) - return errors.Trace(err) - } - } - return nil -} - -func gcv2NotificationActions(role config.ExternalWorkloadRole) (register, recycle bool) { - switch role { - case config.RoleMaster, config.RoleTTLTaskWorker: - return true, true case config.RoleGCV2Worker: - return false, true default: - return false, false + return nil } + + if err := mgr.RecycleGCV2(ctx, safePoint); err != nil { + logutil.Logger(ctx).Error("failed to recycle GCV2 task", + zap.String("category", "gc worker"), + zap.Uint64("safePoint", safePoint), + zap.Error(err)) + return errors.Trace(err) + } + return nil } // deleteRanges processes all delete range records whose ts < safePoint in table `gc_delete_range` diff --git a/pkg/store/gcworker/gc_worker_test.go b/pkg/store/gcworker/gc_worker_test.go index cda4d854ab371..72ca5d6e3004a 100644 --- a/pkg/store/gcworker/gc_worker_test.go +++ b/pkg/store/gcworker/gc_worker_test.go @@ -1178,45 +1178,6 @@ func TestUnifiedGCNeedsToWait(t *testing.T) { } } -func TestGCV2NotificationActions(t *testing.T) { - cases := []struct { - name string - role config.ExternalWorkloadRole - register bool - recycle bool - }{ - { - name: "master", - role: config.RoleMaster, - register: true, - recycle: true, - }, - { - name: "ttl worker", - role: config.RoleTTLTaskWorker, - register: true, - recycle: true, - }, - { - name: "gcv2 worker", - role: config.RoleGCV2Worker, - recycle: true, - }, - { - name: "auto analyze worker", - role: config.RoleAutoAnalyzeWorker, - }, - } - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - register, recycle := gcv2NotificationActions(tc.role) - require.Equal(t, tc.register, register) - require.Equal(t, tc.recycle, recycle) - }) - } -} - func TestResolveLockRangeInfine(t *testing.T) { s := createGCWorkerSuite(t) From 5aa33903808cc75775d18c4e37d011c58840c198 Mon Sep 17 00:00:00 2001 From: AmoebaProtozoa <8039876+AmoebaProtozoa@users.noreply.github.com> Date: Tue, 30 Jun 2026 11:21:14 +0800 Subject: [PATCH 20/28] external workload: use explicit GCV2 role checks --- pkg/store/gcworker/gc_worker.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/pkg/store/gcworker/gc_worker.go b/pkg/store/gcworker/gc_worker.go index b31a3a23cddbc..f85da5a2d11d1 100644 --- a/pkg/store/gcworker/gc_worker.go +++ b/pkg/store/gcworker/gc_worker.go @@ -31,7 +31,6 @@ import ( "github.com/pingcap/kvproto/pkg/keyspacepb" "github.com/pingcap/kvproto/pkg/kvrpcpb" "github.com/pingcap/kvproto/pkg/metapb" - "github.com/pingcap/tidb/pkg/config" "github.com/pingcap/tidb/pkg/config/deploymode" "github.com/pingcap/tidb/pkg/ddl" "github.com/pingcap/tidb/pkg/ddl/label" @@ -854,8 +853,13 @@ func (w *GCWorker) notifyGCV2AfterGC(ctx context.Context, safePoint uint64) erro return nil } - switch mgr.Role() { - case config.RoleMaster, config.RoleTTLTaskWorker: + shouldRegister := extworkload.IsMaster(mgr) || extworkload.IsTTLTaskWorker(mgr) + shouldRecycle := shouldRegister || extworkload.IsGCV2Worker(mgr) + if !shouldRecycle { + return nil + } + + if shouldRegister { gcLifeTime, err := w.loadDurationWithDefault(gcLifeTimeKey, gcDefaultLifeTime) if err != nil { logutil.Logger(ctx).Error("failed to load GC life time for external workload", @@ -870,9 +874,6 @@ func (w *GCWorker) notifyGCV2AfterGC(ctx context.Context, safePoint uint64) erro zap.Error(err)) return errors.Trace(err) } - case config.RoleGCV2Worker: - default: - return nil } if err := mgr.RecycleGCV2(ctx, safePoint); err != nil { From f60833f5bb8a5f2d0fcd0de20c82f838fb1fd275 Mon Sep 17 00:00:00 2001 From: AmoebaProtozoa <8039876+AmoebaProtozoa@users.noreply.github.com> Date: Tue, 30 Jun 2026 11:41:21 +0800 Subject: [PATCH 21/28] external workload: move GCV2 abort helper --- cmd/tidb-server/main.go | 2 -- pkg/extworkload/BUILD.bazel | 2 ++ .../gcv2.go} | 11 +++++---- pkg/session/BUILD.bazel | 1 - pkg/session/session.go | 5 ++-- pkg/session/upgrade_run.go | 3 +++ pkg/store/gcworker/gc_worker.go | 24 +++++++++---------- 7 files changed, 24 insertions(+), 24 deletions(-) rename pkg/{session/external_workload.go => extworkload/gcv2.go} (78%) diff --git a/cmd/tidb-server/main.go b/cmd/tidb-server/main.go index 9f33ea52389a2..077669d585858 100644 --- a/cmd/tidb-server/main.go +++ b/cmd/tidb-server/main.go @@ -661,12 +661,10 @@ func createStoreDDLOwnerMgrAndDomain(keyspaceName string) (kv.Storage, *domain.D // Bootstrap a session to load information schema. err := ddl.StartOwnerManager(context.Background(), storage) if err != nil { - closeExternalWorkloadManager(storage, externalWorkloadManager) return nil, nil, err } dom, err := session.BootstrapSession(storage) if err != nil { - closeExternalWorkloadManager(storage, externalWorkloadManager) return nil, nil, err } return storage, dom, nil diff --git a/pkg/extworkload/BUILD.bazel b/pkg/extworkload/BUILD.bazel index ae7590a1d0304..3a64fd430acbf 100644 --- a/pkg/extworkload/BUILD.bazel +++ b/pkg/extworkload/BUILD.bazel @@ -4,6 +4,7 @@ go_library( name = "extworkload", srcs = [ "external_workload.go", + "gcv2.go", "manager.go", "util.go", ], @@ -14,6 +15,7 @@ go_library( "//pkg/extworkload/client", "//pkg/kv", "//pkg/metrics", + "//pkg/util/intest", "//pkg/util/logutil", "@com_github_pingcap_errors//:errors", "@com_github_pingcap_kvproto//pkg/keyspacepb", diff --git a/pkg/session/external_workload.go b/pkg/extworkload/gcv2.go similarity index 78% rename from pkg/session/external_workload.go rename to pkg/extworkload/gcv2.go index ba216f130e27f..9dd1e764b7a20 100644 --- a/pkg/session/external_workload.go +++ b/pkg/extworkload/gcv2.go @@ -12,22 +12,23 @@ // See the License for the specific language governing permissions and // limitations under the License. -package session +package extworkload import ( "context" "os" - "github.com/pingcap/tidb/pkg/extworkload" "github.com/pingcap/tidb/pkg/kv" "github.com/pingcap/tidb/pkg/util/intest" "github.com/pingcap/tidb/pkg/util/logutil" "go.uber.org/zap" ) -func abortGCV2(store kv.Storage) { - mgr := extworkload.GetManagerFromStore(store) - if !extworkload.IsGCV2Worker(mgr) { +// AbortGCV2ForUpgrade aborts GCV2 work and exits when this TiDB is the dedicated +// GCV2 worker, which must not run normal bootstrap upgrade work. +func AbortGCV2ForUpgrade(store kv.Storage) { + mgr := GetManagerFromStore(store) + if !IsGCV2Worker(mgr) { return } if err := mgr.AbortGCV2(context.Background()); err != nil { diff --git a/pkg/session/BUILD.bazel b/pkg/session/BUILD.bazel index b897c78801be6..f7d98732abf0b 100644 --- a/pkg/session/BUILD.bazel +++ b/pkg/session/BUILD.bazel @@ -6,7 +6,6 @@ go_library( "advisory_locks.go", "bootstrap.go", "contextimpl.go", - "external_workload.go", "mock_bootstrap.go", "nontransactional.go", "session.go", diff --git a/pkg/session/session.go b/pkg/session/session.go index 6468e7a97a27d..bfb9091388c48 100644 --- a/pkg/session/session.go +++ b/pkg/session/session.go @@ -66,6 +66,7 @@ import ( "github.com/pingcap/tidb/pkg/expression/sessionexpr" "github.com/pingcap/tidb/pkg/extension" "github.com/pingcap/tidb/pkg/extension/extensionimpl" + "github.com/pingcap/tidb/pkg/extworkload" "github.com/pingcap/tidb/pkg/infoschema" infoschemactx "github.com/pingcap/tidb/pkg/infoschema/context" "github.com/pingcap/tidb/pkg/infoschema/issyncer" @@ -4596,10 +4597,8 @@ func runInBootstrapSession(store kv.Storage, ver int64) { zap.String("bootMode", string(startMode)), zap.Duration("cost", time.Since(startTime))) }() - if startMode != ddl.Normal { - abortGCV2(store) - } if startMode == ddl.Upgrade { + extworkload.AbortGCV2ForUpgrade(store) // TODO at this time domain must not be created, else it will register server // info, and cause deadlock, we need to make sure this in a clear way logutil.BgLogger().Info("[upgrade] get owner lock to upgrade") diff --git a/pkg/session/upgrade_run.go b/pkg/session/upgrade_run.go index 28db74b0637fc..e170aace006b6 100644 --- a/pkg/session/upgrade_run.go +++ b/pkg/session/upgrade_run.go @@ -18,6 +18,7 @@ import ( "context" "time" + "github.com/pingcap/tidb/pkg/extworkload" "github.com/pingcap/tidb/pkg/kv" "github.com/pingcap/tidb/pkg/meta" "github.com/pingcap/tidb/pkg/parser/terror" @@ -44,6 +45,8 @@ func upgrade(s sessionapi.Session) { return } + extworkload.AbortGCV2ForUpgrade(s.GetStore()) + printClusterState(s, ver) // when upgrade from v6.4.0 or earlier, enables metadata lock automatically, diff --git a/pkg/store/gcworker/gc_worker.go b/pkg/store/gcworker/gc_worker.go index f85da5a2d11d1..3928ab3f93737 100644 --- a/pkg/store/gcworker/gc_worker.go +++ b/pkg/store/gcworker/gc_worker.go @@ -31,6 +31,7 @@ import ( "github.com/pingcap/kvproto/pkg/keyspacepb" "github.com/pingcap/kvproto/pkg/kvrpcpb" "github.com/pingcap/kvproto/pkg/metapb" + "github.com/pingcap/tidb/pkg/config" "github.com/pingcap/tidb/pkg/config/deploymode" "github.com/pingcap/tidb/pkg/ddl" "github.com/pingcap/tidb/pkg/ddl/label" @@ -853,13 +854,8 @@ func (w *GCWorker) notifyGCV2AfterGC(ctx context.Context, safePoint uint64) erro return nil } - shouldRegister := extworkload.IsMaster(mgr) || extworkload.IsTTLTaskWorker(mgr) - shouldRecycle := shouldRegister || extworkload.IsGCV2Worker(mgr) - if !shouldRecycle { - return nil - } - - if shouldRegister { + role := mgr.Role() + if role == config.RoleMaster || role == config.RoleTTLTaskWorker { gcLifeTime, err := w.loadDurationWithDefault(gcLifeTimeKey, gcDefaultLifeTime) if err != nil { logutil.Logger(ctx).Error("failed to load GC life time for external workload", @@ -876,12 +872,14 @@ func (w *GCWorker) notifyGCV2AfterGC(ctx context.Context, safePoint uint64) erro } } - if err := mgr.RecycleGCV2(ctx, safePoint); err != nil { - logutil.Logger(ctx).Error("failed to recycle GCV2 task", - zap.String("category", "gc worker"), - zap.Uint64("safePoint", safePoint), - zap.Error(err)) - return errors.Trace(err) + if role == config.RoleMaster || role == config.RoleTTLTaskWorker || role == config.RoleGCV2Worker { + if err := mgr.RecycleGCV2(ctx, safePoint); err != nil { + logutil.Logger(ctx).Error("failed to recycle GCV2 task", + zap.String("category", "gc worker"), + zap.Uint64("safePoint", safePoint), + zap.Error(err)) + return errors.Trace(err) + } } return nil } From 890ad299ab49610917acf98c3264a76d42c2b179 Mon Sep 17 00:00:00 2001 From: AmoebaProtozoa <8039876+AmoebaProtozoa@users.noreply.github.com> Date: Tue, 30 Jun 2026 15:50:38 +0800 Subject: [PATCH 22/28] external workload: tighten GCV2 wiring --- cmd/tidb-server/main.go | 11 ++--- pkg/store/gcworker/BUILD.bazel | 3 +- pkg/store/gcworker/gc_worker_test.go | 71 ++++++++++++++++++++++++++++ 3 files changed, 77 insertions(+), 8 deletions(-) diff --git a/cmd/tidb-server/main.go b/cmd/tidb-server/main.go index 077669d585858..c326197f96e63 100644 --- a/cmd/tidb-server/main.go +++ b/cmd/tidb-server/main.go @@ -329,8 +329,8 @@ func initExternalWorkloadManager(ctx context.Context, storage kv.Storage) extwor if !cfg.Enable { return nil } - // Regular TiDB roles can continue without coordination, but a dedicated - // GCV2 worker must not run without the controller. + // Non-GCV2 roles can continue without coordination, but a dedicated GCV2 + // worker must not run without the controller. meta := storage.GetCodec().GetKeyspaceMeta() if meta == nil { if cfg.Role == config.RoleGCV2Worker { @@ -497,9 +497,7 @@ func main() { terror.MustNil(err) repository.SetupRepository(dom) if externalWorkloadManager := extworkload.GetManagerFromStore(storage); externalWorkloadManager != nil { - defer func() { - closeExternalWorkloadManager(storage, externalWorkloadManager) - }() + defer closeExternalWorkloadManager(storage, externalWorkloadManager) } svr := createServer(storage, dom) if standbyController != nil { @@ -651,9 +649,8 @@ func createStoreDDLOwnerMgrAndDomain(keyspaceName string) (kv.Storage, *domain.D externalWorkloadManager := initExternalWorkloadManager(context.Background(), storage) if extworkload.IsMaster(externalWorkloadManager) && pd.IsKeyspaceUsingKeyspaceLevelGC(externalWorkloadManager.Meta()) { if err := externalWorkloadManager.InitializeGCV2(context.Background()); err != nil { - logutil.BgLogger().Warn("failed to initialize external workload service; TiDB will continue without external workload coordination", zap.Error(err)) + logutil.BgLogger().Warn("failed to initialize external workload GCV2 task; TiDB will continue without external workload coordination", zap.Error(err)) closeExternalWorkloadManager(storage, externalWorkloadManager) - externalWorkloadManager = nil } } copr.GlobalMPPFailedStoreProber.Run() diff --git a/pkg/store/gcworker/BUILD.bazel b/pkg/store/gcworker/BUILD.bazel index c9affc9ac40de..c2b5521265669 100644 --- a/pkg/store/gcworker/BUILD.bazel +++ b/pkg/store/gcworker/BUILD.bazel @@ -60,7 +60,7 @@ go_test( embed = [":gcworker"], flaky = True, race = "on", - shard_count = 37, + shard_count = 38, deps = [ "//pkg/config", "//pkg/config/deploymode", @@ -69,6 +69,7 @@ go_test( "//pkg/ddl/util", "//pkg/domain", "//pkg/domain/infosync", + "//pkg/extworkload", "//pkg/kv", "//pkg/meta/model", "//pkg/session", diff --git a/pkg/store/gcworker/gc_worker_test.go b/pkg/store/gcworker/gc_worker_test.go index 72ca5d6e3004a..170a90551761b 100644 --- a/pkg/store/gcworker/gc_worker_test.go +++ b/pkg/store/gcworker/gc_worker_test.go @@ -41,6 +41,7 @@ import ( "github.com/pingcap/tidb/pkg/ddl/util" "github.com/pingcap/tidb/pkg/domain" "github.com/pingcap/tidb/pkg/domain/infosync" + "github.com/pingcap/tidb/pkg/extworkload" "github.com/pingcap/tidb/pkg/kv" "github.com/pingcap/tidb/pkg/meta/model" "github.com/pingcap/tidb/pkg/session" @@ -68,6 +69,30 @@ type mockGCWorkerLockResolver struct { batchResolveLocks func([]*txnlock.Lock, *tikv.KeyLocation) (*tikv.KeyLocation, error) } +type stubGCV2Manager struct { + extworkload.Manager + + role config.ExternalWorkloadRole + meta *keyspacepb.KeyspaceMeta + recycleSafePoint uint64 + registerCount int + recycleCount int +} + +func (m *stubGCV2Manager) Role() config.ExternalWorkloadRole { return m.role } +func (m *stubGCV2Manager) Meta() *keyspacepb.KeyspaceMeta { return m.meta } + +func (m *stubGCV2Manager) RegisterGCV2(_ context.Context, _ uint64, _ int64) error { + m.registerCount++ + return nil +} + +func (m *stubGCV2Manager) RecycleGCV2(_ context.Context, safePoint uint64) error { + m.recycleSafePoint = safePoint + m.recycleCount++ + return nil +} + func (l *mockGCWorkerLockResolver) ScanLocksInOneRegion(bo *tikv.Backoffer, key []byte, endKey []byte, maxVersion uint64, limit uint32) ([]*txnlock.Lock, *tikv.KeyLocation, error) { locks, loc, err := l.RegionLockResolver.ScanLocksInOneRegion(bo, key, endKey, maxVersion, limit) if err != nil { @@ -224,6 +249,52 @@ func createGCWorkerSuite(t *testing.T, opts ...mockGCWorkerSuiteOption) *mockGCW return s } +func TestNotifyGCV2AfterGCForDedicatedWorker(t *testing.T) { + const safePoint = 123 + for _, tc := range []struct { + name string + gcManagementType string + wantRecycleCount int + }{ + { + name: "keyspace level GC", + gcManagementType: pd.KeyspaceConfigGCManagementTypeKeyspaceLevel, + wantRecycleCount: 1, + }, + { + name: "unified GC", + gcManagementType: pd.KeyspaceConfigGCManagementTypeUnified, + wantRecycleCount: 0, + }, + } { + t.Run(tc.name, func(t *testing.T) { + store, err := mockstore.NewMockStore(mockstore.WithCurrentKeyspaceMeta(&keyspacepb.KeyspaceMeta{ + Id: 1, + Name: "ks", + Config: map[string]string{pd.KeyspaceConfigGCManagementType: tc.gcManagementType}, + })) + require.NoError(t, err) + t.Cleanup(func() { + require.NoError(t, store.Close()) + }) + + mgr := &stubGCV2Manager{ + role: config.RoleGCV2Worker, + meta: store.GetCodec().GetKeyspaceMeta(), + } + extworkload.SetManagerForStore(store, mgr) + worker := &GCWorker{store: store} + + require.NoError(t, worker.notifyGCV2AfterGC(context.Background(), safePoint)) + require.Zero(t, mgr.registerCount) + require.Equal(t, tc.wantRecycleCount, mgr.recycleCount) + if tc.wantRecycleCount > 0 { + require.Equal(t, uint64(safePoint), mgr.recycleSafePoint) + } + }) + } +} + func (s *mockGCWorkerSuite) mustPut(t *testing.T, key, value string) { txn, err := s.store.Begin() require.NoError(t, err) From b677f79ba7ff2983d20c3f68a9d168982d048cfa Mon Sep 17 00:00:00 2001 From: AmoebaProtozoa <8039876+AmoebaProtozoa@users.noreply.github.com> Date: Tue, 30 Jun 2026 16:24:41 +0800 Subject: [PATCH 23/28] external workload: fold GCV2 abort utility --- pkg/extworkload/BUILD.bazel | 1 - pkg/extworkload/gcv2.go | 42 ------------------------------------- pkg/extworkload/util.go | 23 ++++++++++++++++++++ 3 files changed, 23 insertions(+), 43 deletions(-) delete mode 100644 pkg/extworkload/gcv2.go diff --git a/pkg/extworkload/BUILD.bazel b/pkg/extworkload/BUILD.bazel index 3a64fd430acbf..cbb4e77c10789 100644 --- a/pkg/extworkload/BUILD.bazel +++ b/pkg/extworkload/BUILD.bazel @@ -4,7 +4,6 @@ go_library( name = "extworkload", srcs = [ "external_workload.go", - "gcv2.go", "manager.go", "util.go", ], diff --git a/pkg/extworkload/gcv2.go b/pkg/extworkload/gcv2.go deleted file mode 100644 index 9dd1e764b7a20..0000000000000 --- a/pkg/extworkload/gcv2.go +++ /dev/null @@ -1,42 +0,0 @@ -// Copyright 2026 PingCAP, Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package extworkload - -import ( - "context" - "os" - - "github.com/pingcap/tidb/pkg/kv" - "github.com/pingcap/tidb/pkg/util/intest" - "github.com/pingcap/tidb/pkg/util/logutil" - "go.uber.org/zap" -) - -// AbortGCV2ForUpgrade aborts GCV2 work and exits when this TiDB is the dedicated -// GCV2 worker, which must not run normal bootstrap upgrade work. -func AbortGCV2ForUpgrade(store kv.Storage) { - mgr := GetManagerFromStore(store) - if !IsGCV2Worker(mgr) { - return - } - if err := mgr.AbortGCV2(context.Background()); err != nil { - logutil.BgLogger().Fatal("abort GCV2 worker failed", zap.Error(err)) - } - logutil.BgLogger().Info("GCV2 worker aborted") - if intest.InTest { - return - } - os.Exit(0) -} diff --git a/pkg/extworkload/util.go b/pkg/extworkload/util.go index 47e155a50f0a3..37fdcd650c52b 100644 --- a/pkg/extworkload/util.go +++ b/pkg/extworkload/util.go @@ -15,8 +15,14 @@ package extworkload import ( + "context" + "os" + "github.com/pingcap/tidb/pkg/config" "github.com/pingcap/tidb/pkg/kv" + "github.com/pingcap/tidb/pkg/util/intest" + "github.com/pingcap/tidb/pkg/util/logutil" + "go.uber.org/zap" ) // IsEnabled reports whether a Manager is present. @@ -57,6 +63,23 @@ func GetManagerFromStore(store kv.Storage) Manager { return mgr } +// AbortGCV2ForUpgrade aborts GCV2 work and exits when this TiDB is the dedicated +// GCV2 worker, which must not run normal bootstrap upgrade work. +func AbortGCV2ForUpgrade(store kv.Storage) { + mgr := GetManagerFromStore(store) + if !IsGCV2Worker(mgr) { + return + } + if err := mgr.AbortGCV2(context.Background()); err != nil { + logutil.BgLogger().Fatal("abort GCV2 worker failed", zap.Error(err)) + } + logutil.BgLogger().Info("GCV2 worker aborted") + if intest.InTest { + return + } + os.Exit(0) +} + func roleIs(m Manager, role config.ExternalWorkloadRole) bool { return IsEnabled(m) && m.Role() == role } From ab0e556f99045b11cdcbb9d1bf41ab5ac46efa4a Mon Sep 17 00:00:00 2001 From: AmoebaProtozoa <8039876+AmoebaProtozoa@users.noreply.github.com> Date: Tue, 30 Jun 2026 17:17:15 +0800 Subject: [PATCH 24/28] external workload: make GCV2 notification best effort --- pkg/store/gcworker/gc_worker.go | 23 +++++++---------------- pkg/store/gcworker/gc_worker_test.go | 15 ++++++++++++--- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/pkg/store/gcworker/gc_worker.go b/pkg/store/gcworker/gc_worker.go index 3928ab3f93737..7dd15396e1fcc 100644 --- a/pkg/store/gcworker/gc_worker.go +++ b/pkg/store/gcworker/gc_worker.go @@ -839,49 +839,40 @@ func (w *GCWorker) runGCJob(ctx context.Context, safePoint uint64, concurrency g return errors.Trace(err) } - err = w.notifyGCV2AfterGC(ctx, gcSafePoint) - if err != nil { - return errors.Trace(err) - } - + w.notifyGCV2AfterGC(ctx, gcSafePoint) metrics.GCHistogram.WithLabelValues(metrics.StageTotal).Observe(time.Since(startTime).Seconds()) return nil } -func (w *GCWorker) notifyGCV2AfterGC(ctx context.Context, safePoint uint64) error { +func (w *GCWorker) notifyGCV2AfterGC(ctx context.Context, safePoint uint64) { mgr := extworkload.GetManagerFromStore(w.store) if !extworkload.IsEnabled(mgr) || !pd.IsKeyspaceUsingKeyspaceLevelGC(mgr.Meta()) { - return nil + return } role := mgr.Role() if role == config.RoleMaster || role == config.RoleTTLTaskWorker { gcLifeTime, err := w.loadDurationWithDefault(gcLifeTimeKey, gcDefaultLifeTime) if err != nil { - logutil.Logger(ctx).Error("failed to load GC life time for external workload", + logutil.Logger(ctx).Warn("failed to load GC life time for external workload", zap.String("category", "gc worker"), zap.Error(err)) - return errors.Trace(err) - } - if err := mgr.RegisterGCV2(ctx, safePoint, int64(*gcLifeTime/time.Second)); err != nil { - logutil.Logger(ctx).Error("failed to register GCV2 task", + } else if err := mgr.RegisterGCV2(ctx, safePoint, int64(*gcLifeTime/time.Second)); err != nil { + logutil.Logger(ctx).Warn("failed to register GCV2 task", zap.String("category", "gc worker"), zap.Uint64("safePoint", safePoint), zap.Error(err)) - return errors.Trace(err) } } if role == config.RoleMaster || role == config.RoleTTLTaskWorker || role == config.RoleGCV2Worker { if err := mgr.RecycleGCV2(ctx, safePoint); err != nil { - logutil.Logger(ctx).Error("failed to recycle GCV2 task", + logutil.Logger(ctx).Warn("failed to recycle GCV2 task", zap.String("category", "gc worker"), zap.Uint64("safePoint", safePoint), zap.Error(err)) - return errors.Trace(err) } } - return nil } // deleteRanges processes all delete range records whose ts < safePoint in table `gc_delete_range` diff --git a/pkg/store/gcworker/gc_worker_test.go b/pkg/store/gcworker/gc_worker_test.go index 170a90551761b..57e352bbf20ee 100644 --- a/pkg/store/gcworker/gc_worker_test.go +++ b/pkg/store/gcworker/gc_worker_test.go @@ -74,6 +74,7 @@ type stubGCV2Manager struct { role config.ExternalWorkloadRole meta *keyspacepb.KeyspaceMeta + err error recycleSafePoint uint64 registerCount int recycleCount int @@ -84,13 +85,13 @@ func (m *stubGCV2Manager) Meta() *keyspacepb.KeyspaceMeta { return m.meta } func (m *stubGCV2Manager) RegisterGCV2(_ context.Context, _ uint64, _ int64) error { m.registerCount++ - return nil + return m.err } func (m *stubGCV2Manager) RecycleGCV2(_ context.Context, safePoint uint64) error { m.recycleSafePoint = safePoint m.recycleCount++ - return nil + return m.err } func (l *mockGCWorkerLockResolver) ScanLocksInOneRegion(bo *tikv.Backoffer, key []byte, endKey []byte, maxVersion uint64, limit uint32) ([]*txnlock.Lock, *tikv.KeyLocation, error) { @@ -254,6 +255,7 @@ func TestNotifyGCV2AfterGCForDedicatedWorker(t *testing.T) { for _, tc := range []struct { name string gcManagementType string + recycleErr error wantRecycleCount int }{ { @@ -261,6 +263,12 @@ func TestNotifyGCV2AfterGCForDedicatedWorker(t *testing.T) { gcManagementType: pd.KeyspaceConfigGCManagementTypeKeyspaceLevel, wantRecycleCount: 1, }, + { + name: "recycle failure is best effort", + gcManagementType: pd.KeyspaceConfigGCManagementTypeKeyspaceLevel, + recycleErr: errors.New("mock recycle GCV2 failure"), + wantRecycleCount: 1, + }, { name: "unified GC", gcManagementType: pd.KeyspaceConfigGCManagementTypeUnified, @@ -281,11 +289,12 @@ func TestNotifyGCV2AfterGCForDedicatedWorker(t *testing.T) { mgr := &stubGCV2Manager{ role: config.RoleGCV2Worker, meta: store.GetCodec().GetKeyspaceMeta(), + err: tc.recycleErr, } extworkload.SetManagerForStore(store, mgr) worker := &GCWorker{store: store} - require.NoError(t, worker.notifyGCV2AfterGC(context.Background(), safePoint)) + worker.notifyGCV2AfterGC(context.Background(), safePoint) require.Zero(t, mgr.registerCount) require.Equal(t, tc.wantRecycleCount, mgr.recycleCount) if tc.wantRecycleCount > 0 { From 848d589a96a6a120a090a752e3d28036ee4351c9 Mon Sep 17 00:00:00 2001 From: AmoebaProtozoa <8039876+AmoebaProtozoa@users.noreply.github.com> Date: Thu, 2 Jul 2026 11:30:23 +0800 Subject: [PATCH 25/28] external workload: address GCV2 review feedback --- pkg/executor/BUILD.bazel | 1 + pkg/executor/set.go | 17 +++++++++----- pkg/executor/set_test.go | 40 +++++++++++++++++++++++++++++++++ pkg/store/gcworker/gc_worker.go | 18 +++++++-------- 4 files changed, 62 insertions(+), 14 deletions(-) diff --git a/pkg/executor/BUILD.bazel b/pkg/executor/BUILD.bazel index 0cf34e1a0cf73..dfaa693142ff2 100644 --- a/pkg/executor/BUILD.bazel +++ b/pkg/executor/BUILD.bazel @@ -451,6 +451,7 @@ go_test( "//pkg/expression/aggregation", "//pkg/expression/exprstatic", "//pkg/extension", + "//pkg/extworkload", "//pkg/infoschema", "//pkg/keyspace", "//pkg/kv", diff --git a/pkg/executor/set.go b/pkg/executor/set.go index 92dfae7b50716..fc998299f095c 100644 --- a/pkg/executor/set.go +++ b/pkg/executor/set.go @@ -195,15 +195,22 @@ func (e *SetExecutor) setSysVariable(ctx context.Context, name string, v *expres if v.IsGlobal && name == vardef.TiDBGCLifetime { mgr := extworkload.GetManagerFromStore(e.Ctx().GetStore()) if extworkload.IsMaster(mgr) && pd.IsKeyspaceUsingKeyspaceLevelGC(mgr.Meta()) { - gcLifeTime, err := time.ParseDuration(valStr) + gcLifeTimeVal, err := sessionVars.GlobalVarsAccessor.GetGlobalSysVar(name) if err != nil { - return err - } - if err = mgr.UpdateGCLifeTime(ctx, int64(gcLifeTime/time.Second)); err != nil { - logutil.BgLogger().Warn("failed to update external workload GC life time", + logutil.BgLogger().Warn("failed to load effective external workload GC life time", zap.String("name", name), zap.String("val", showValStr), zap.Error(err)) + } else if gcLifeTime, err := time.ParseDuration(gcLifeTimeVal); err != nil { + logutil.BgLogger().Warn("failed to parse effective external workload GC life time", + zap.String("name", name), + zap.String("val", gcLifeTimeVal), + zap.Error(err)) + } else if err = mgr.UpdateGCLifeTime(ctx, int64(gcLifeTime/time.Second)); err != nil { + logutil.BgLogger().Warn("failed to update external workload GC life time", + zap.String("name", name), + zap.String("val", gcLifeTimeVal), + zap.Error(err)) } } } diff --git a/pkg/executor/set_test.go b/pkg/executor/set_test.go index 1cb6d0f669e0a..670bc8e541532 100644 --- a/pkg/executor/set_test.go +++ b/pkg/executor/set_test.go @@ -25,10 +25,12 @@ import ( "testing" "github.com/pingcap/failpoint" + "github.com/pingcap/kvproto/pkg/keyspacepb" "github.com/pingcap/tidb/pkg/config" "github.com/pingcap/tidb/pkg/errno" "github.com/pingcap/tidb/pkg/executor" "github.com/pingcap/tidb/pkg/expression" + "github.com/pingcap/tidb/pkg/extworkload" "github.com/pingcap/tidb/pkg/infoschema" "github.com/pingcap/tidb/pkg/parser/mysql" "github.com/pingcap/tidb/pkg/parser/terror" @@ -40,8 +42,46 @@ import ( "github.com/pingcap/tidb/pkg/util/mock" topsqlstate "github.com/pingcap/tidb/pkg/util/topsql/state" "github.com/stretchr/testify/require" + pd "github.com/tikv/pd/client" ) +type setGCLifeTimeManager struct { + extworkload.Manager + gcLifeTime int64 + updateCnt int +} + +func (m *setGCLifeTimeManager) Role() config.ExternalWorkloadRole { + return config.RoleMaster +} + +func (*setGCLifeTimeManager) Meta() *keyspacepb.KeyspaceMeta { + return &keyspacepb.KeyspaceMeta{ + Config: map[string]string{ + pd.KeyspaceConfigGCManagementType: pd.KeyspaceConfigGCManagementTypeKeyspaceLevel, + }, + } +} + +func (m *setGCLifeTimeManager) UpdateGCLifeTime(_ context.Context, gcLifeTime int64) error { + m.updateCnt++ + m.gcLifeTime = gcLifeTime + return nil +} + +func TestSetGCLifeTimeNotifiesExternalWorkloadWithEffectiveValue(t *testing.T) { + store := testkit.CreateMockStore(t) + mgr := &setGCLifeTimeManager{} + extworkload.SetManagerForStore(store, mgr) + + tk := testkit.NewTestKit(t, store) + tk.MustExec("set global tidb_gc_life_time = '1m'") + + tk.MustQuery("select @@global.tidb_gc_life_time").Check(testkit.Rows("10m0s")) + require.Equal(t, 1, mgr.updateCnt) + require.Equal(t, int64(600), mgr.gcLifeTime) +} + func TestSetVar(t *testing.T) { store := testkit.CreateMockStore(t) tk := testkit.NewTestKit(t, store) diff --git a/pkg/store/gcworker/gc_worker.go b/pkg/store/gcworker/gc_worker.go index 7dd15396e1fcc..ad8eb26bf2899 100644 --- a/pkg/store/gcworker/gc_worker.go +++ b/pkg/store/gcworker/gc_worker.go @@ -851,6 +851,15 @@ func (w *GCWorker) notifyGCV2AfterGC(ctx context.Context, safePoint uint64) { } role := mgr.Role() + if role == config.RoleMaster || role == config.RoleTTLTaskWorker || role == config.RoleGCV2Worker { + if err := mgr.RecycleGCV2(ctx, safePoint); err != nil { + logutil.Logger(ctx).Warn("failed to recycle GCV2 task", + zap.String("category", "gc worker"), + zap.Uint64("safePoint", safePoint), + zap.Error(err)) + } + } + if role == config.RoleMaster || role == config.RoleTTLTaskWorker { gcLifeTime, err := w.loadDurationWithDefault(gcLifeTimeKey, gcDefaultLifeTime) if err != nil { @@ -864,15 +873,6 @@ func (w *GCWorker) notifyGCV2AfterGC(ctx context.Context, safePoint uint64) { zap.Error(err)) } } - - if role == config.RoleMaster || role == config.RoleTTLTaskWorker || role == config.RoleGCV2Worker { - if err := mgr.RecycleGCV2(ctx, safePoint); err != nil { - logutil.Logger(ctx).Warn("failed to recycle GCV2 task", - zap.String("category", "gc worker"), - zap.Uint64("safePoint", safePoint), - zap.Error(err)) - } - } } // deleteRanges processes all delete range records whose ts < safePoint in table `gc_delete_range` From 24c71dea8fd1eea17721f8682cbe7c65763aaf20 Mon Sep 17 00:00:00 2001 From: AmoebaProtozoa <8039876+AmoebaProtozoa@users.noreply.github.com> Date: Wed, 8 Jul 2026 10:37:50 +0800 Subject: [PATCH 26/28] external workload: seed GCV2 with effective GC lifetime --- cmd/tidb-server/BUILD.bazel | 6 +++- cmd/tidb-server/main.go | 40 +++++++++++++++++++++++---- cmd/tidb-server/main_test.go | 41 ++++++++++++++++++++++++++++ pkg/extworkload/external_workload.go | 2 +- pkg/extworkload/manager.go | 8 ++---- pkg/extworkload/manager_test.go | 4 +-- 6 files changed, 85 insertions(+), 16 deletions(-) diff --git a/cmd/tidb-server/BUILD.bazel b/cmd/tidb-server/BUILD.bazel index 107532ee8661d..1f0b1ee0befe1 100644 --- a/cmd/tidb-server/BUILD.bazel +++ b/cmd/tidb-server/BUILD.bazel @@ -112,16 +112,20 @@ go_test( srcs = ["main_test.go"], embed = [":tidb-server_lib"], flaky = True, - shard_count = 11, + shard_count = 12, deps = [ "//pkg/config", "//pkg/config/deploymode", "//pkg/config/kerneltype", + "//pkg/extworkload", "//pkg/parser/mysql", "//pkg/sessionctx/vardef", "//pkg/sessionctx/variable", + "//pkg/testkit", "//pkg/testkit/testsetup", + "@com_github_pingcap_kvproto//pkg/keyspacepb", "@com_github_stretchr_testify//require", + "@com_github_tikv_pd_client//:client", "@io_opencensus_go//stats/view", "@org_uber_go_goleak//:goleak", ], diff --git a/cmd/tidb-server/main.go b/cmd/tidb-server/main.go index c326197f96e63..aae5fdc36ab35 100644 --- a/cmd/tidb-server/main.go +++ b/cmd/tidb-server/main.go @@ -364,6 +364,39 @@ func closeExternalWorkloadManager(storage kv.Storage, mgr extworkload.Manager) { } } +func loadExternalWorkloadGCLifeTime(ctx context.Context, storage kv.Storage) (time.Duration, error) { + se, err := session.CreateSession(storage) + if err != nil { + return 0, err + } + defer se.Close() + gcLifeTimeVal, err := variable.GetSysVar(vardef.TiDBGCLifetime).GetGlobalFromHook(ctx, se.GetSessionVars()) + if err != nil { + return 0, err + } + gcLifeTime, err := time.ParseDuration(gcLifeTimeVal) + if err != nil { + return 0, err + } + return gcLifeTime, nil +} + +func initializeExternalWorkloadGCV2(ctx context.Context, storage kv.Storage, mgr extworkload.Manager) { + if !extworkload.IsMaster(mgr) || !pd.IsKeyspaceUsingKeyspaceLevelGC(mgr.Meta()) { + return + } + gcLifeTime, err := loadExternalWorkloadGCLifeTime(ctx, storage) + if err != nil { + logutil.BgLogger().Warn("failed to load GC life time for external workload GCV2 task; TiDB will continue without external workload coordination", zap.Error(err)) + closeExternalWorkloadManager(storage, mgr) + return + } + if err := mgr.InitializeGCV2(ctx, int64(gcLifeTime/time.Second)); err != nil { + logutil.BgLogger().Warn("failed to initialize external workload GCV2 task; TiDB will continue without external workload coordination", zap.Error(err)) + closeExternalWorkloadManager(storage, mgr) + } +} + func main() { fset := initFlagSet() if args := fset.Args(); len(args) != 0 { @@ -647,12 +680,6 @@ func createStoreDDLOwnerMgrAndDomain(keyspaceName string) (kv.Storage, *domain.D } } externalWorkloadManager := initExternalWorkloadManager(context.Background(), storage) - if extworkload.IsMaster(externalWorkloadManager) && pd.IsKeyspaceUsingKeyspaceLevelGC(externalWorkloadManager.Meta()) { - if err := externalWorkloadManager.InitializeGCV2(context.Background()); err != nil { - logutil.BgLogger().Warn("failed to initialize external workload GCV2 task; TiDB will continue without external workload coordination", zap.Error(err)) - closeExternalWorkloadManager(storage, externalWorkloadManager) - } - } copr.GlobalMPPFailedStoreProber.Run() mppcoordmanager.InstanceMPPCoordinatorManager.Run() // Bootstrap a session to load information schema. @@ -664,6 +691,7 @@ func createStoreDDLOwnerMgrAndDomain(keyspaceName string) (kv.Storage, *domain.D if err != nil { return nil, nil, err } + initializeExternalWorkloadGCV2(context.Background(), storage, externalWorkloadManager) return storage, dom, nil } diff --git a/cmd/tidb-server/main_test.go b/cmd/tidb-server/main_test.go index 5808ccc14bdf9..eef31919fb51f 100644 --- a/cmd/tidb-server/main_test.go +++ b/cmd/tidb-server/main_test.go @@ -15,18 +15,23 @@ package main import ( + "context" "os" "syscall" "testing" + "github.com/pingcap/kvproto/pkg/keyspacepb" "github.com/pingcap/tidb/pkg/config" "github.com/pingcap/tidb/pkg/config/deploymode" "github.com/pingcap/tidb/pkg/config/kerneltype" + "github.com/pingcap/tidb/pkg/extworkload" "github.com/pingcap/tidb/pkg/parser/mysql" "github.com/pingcap/tidb/pkg/sessionctx/vardef" "github.com/pingcap/tidb/pkg/sessionctx/variable" + "github.com/pingcap/tidb/pkg/testkit" "github.com/pingcap/tidb/pkg/testkit/testsetup" "github.com/stretchr/testify/require" + pd "github.com/tikv/pd/client" "go.opencensus.io/stats/view" "go.uber.org/goleak" ) @@ -182,6 +187,42 @@ func TestInitDeployMode(t *testing.T) { }) } +type gcv2InitManager struct { + extworkload.Manager + gcLifeTime int64 + initCnt int +} + +func (m *gcv2InitManager) Role() config.ExternalWorkloadRole { + return config.RoleMaster +} + +func (*gcv2InitManager) Meta() *keyspacepb.KeyspaceMeta { + return &keyspacepb.KeyspaceMeta{ + Config: map[string]string{ + pd.KeyspaceConfigGCManagementType: pd.KeyspaceConfigGCManagementTypeKeyspaceLevel, + }, + } +} + +func (m *gcv2InitManager) InitializeGCV2(_ context.Context, gcLifeTime int64) error { + m.initCnt++ + m.gcLifeTime = gcLifeTime + return nil +} + +func TestInitializeExternalWorkloadGCV2UsesEffectiveGCLifeTime(t *testing.T) { + store := testkit.CreateMockStore(t) + tk := testkit.NewTestKit(t, store) + tk.MustExec("set global tidb_gc_life_time = '24h'") + + mgr := &gcv2InitManager{} + initializeExternalWorkloadGCV2(context.Background(), store, mgr) + + require.Equal(t, 1, mgr.initCnt) + require.Equal(t, int64(24*60*60), mgr.gcLifeTime) +} + func TestCreateMgrClientRequiresPodIdentityInStarter(t *testing.T) { if kerneltype.IsClassic() { t.Skip("only for nextgen kernel") diff --git a/pkg/extworkload/external_workload.go b/pkg/extworkload/external_workload.go index 53514ae617a1e..c3e33de39edba 100644 --- a/pkg/extworkload/external_workload.go +++ b/pkg/extworkload/external_workload.go @@ -35,7 +35,7 @@ type Manager interface { Meta() *keyspacepb.KeyspaceMeta // InitializeGCV2 seeds the controller with an initial keyspace-level GC task. - InitializeGCV2(ctx context.Context) error + InitializeGCV2(ctx context.Context, gcLifeTime int64) error // AbortGCV2 asks the controller to abort all outstanding keyspace-level GC tasks. AbortGCV2(ctx context.Context) error // RegisterGCV2 reports that a keyspace-level GC round at safePoint has completed. diff --git a/pkg/extworkload/manager.go b/pkg/extworkload/manager.go index 7f699afbeeb18..7fa378d19d810 100644 --- a/pkg/extworkload/manager.go +++ b/pkg/extworkload/manager.go @@ -30,10 +30,6 @@ import ( "google.golang.org/grpc" ) -// defGCLifeTimeSec is the gc_life_time (seconds) seeded to the controller -// at InitializeGCV2 time, before the local gc_life_time variable is loaded. -const defGCLifeTimeSec = 600 - // dialTimeout bounds the synchronous dial + Ping during manager creation. const dialTimeout = 30 * time.Second @@ -138,11 +134,11 @@ func withRequestTimeout(ctx context.Context) (context.Context, context.CancelFun return context.WithTimeout(ctx, requestTimeout) } -func (m *manager) InitializeGCV2(ctx context.Context) error { +func (m *manager) InitializeGCV2(ctx context.Context, gcLifeTime int64) error { ctx, cancel := withRequestTimeout(ctx) defer cancel() ctx = withMetric(ctx, string(config.RoleGCV2Worker), metrics.WorkerActionInit) - return m.cli.RegisterGCV2(ctx, 0, defGCLifeTimeSec) + return m.cli.RegisterGCV2(ctx, 0, gcLifeTime) } func (m *manager) AbortGCV2(ctx context.Context) error { diff --git a/pkg/extworkload/manager_test.go b/pkg/extworkload/manager_test.go index 017ef7cf1faff..7e2d59fd57180 100644 --- a/pkg/extworkload/manager_test.go +++ b/pkg/extworkload/manager_test.go @@ -184,11 +184,11 @@ func TestManagerMethodsSetDeadlineAndMetrics(t *testing.T) { }{ { name: "InitializeGCV2", - call: func(m *manager) error { return m.InitializeGCV2(context.Background()) }, + call: func(m *manager) error { return m.InitializeGCV2(context.Background(), 3600) }, check: func(cli *fakeClient) { require.Equal(t, "RegisterGCV2", cli.call) require.Equal(t, uint64(0), cli.safePoint) - require.Equal(t, int64(defGCLifeTimeSec), cli.gcLifeTime) + require.Equal(t, int64(3600), cli.gcLifeTime) requireLabels(t, cli, string(config.RoleGCV2Worker), metrics.WorkerActionInit) }, }, From 2e3d8d3b52879fa0ee09b92ee3157b96e0ef99ed Mon Sep 17 00:00:00 2001 From: AmoebaProtozoa <8039876+AmoebaProtozoa@users.noreply.github.com> Date: Tue, 14 Jul 2026 14:36:54 +0800 Subject: [PATCH 27/28] external workload: tighten GCV2 upgrade and lifetime handling --- cmd/tidb-server/main.go | 2 +- cmd/tidb-server/main_test.go | 7 ++-- pkg/executor/set.go | 51 ++++++++++++++---------- pkg/executor/set_test.go | 58 +++++++++++++++++++--------- pkg/extworkload/BUILD.bazel | 3 +- pkg/extworkload/external_workload.go | 7 ++-- pkg/extworkload/manager.go | 12 +++--- pkg/extworkload/manager_test.go | 7 ++-- pkg/extworkload/util.go | 23 ++++------- pkg/extworkload/util_test.go | 38 +++++++++++++++++- pkg/session/session.go | 15 ++++++- pkg/session/session_nextgen_test.go | 37 ++++++++++++++++++ pkg/session/upgrade_run.go | 3 -- pkg/store/gcworker/gc_worker.go | 2 +- pkg/store/gcworker/gc_worker_test.go | 2 +- 15 files changed, 188 insertions(+), 79 deletions(-) diff --git a/cmd/tidb-server/main.go b/cmd/tidb-server/main.go index aae5fdc36ab35..2cb170a83c942 100644 --- a/cmd/tidb-server/main.go +++ b/cmd/tidb-server/main.go @@ -391,7 +391,7 @@ func initializeExternalWorkloadGCV2(ctx context.Context, storage kv.Storage, mgr closeExternalWorkloadManager(storage, mgr) return } - if err := mgr.InitializeGCV2(ctx, int64(gcLifeTime/time.Second)); err != nil { + if err := mgr.InitializeGCV2(ctx, gcLifeTime); err != nil { logutil.BgLogger().Warn("failed to initialize external workload GCV2 task; TiDB will continue without external workload coordination", zap.Error(err)) closeExternalWorkloadManager(storage, mgr) } diff --git a/cmd/tidb-server/main_test.go b/cmd/tidb-server/main_test.go index eef31919fb51f..971f5be1959cb 100644 --- a/cmd/tidb-server/main_test.go +++ b/cmd/tidb-server/main_test.go @@ -19,6 +19,7 @@ import ( "os" "syscall" "testing" + "time" "github.com/pingcap/kvproto/pkg/keyspacepb" "github.com/pingcap/tidb/pkg/config" @@ -189,7 +190,7 @@ func TestInitDeployMode(t *testing.T) { type gcv2InitManager struct { extworkload.Manager - gcLifeTime int64 + gcLifeTime time.Duration initCnt int } @@ -205,7 +206,7 @@ func (*gcv2InitManager) Meta() *keyspacepb.KeyspaceMeta { } } -func (m *gcv2InitManager) InitializeGCV2(_ context.Context, gcLifeTime int64) error { +func (m *gcv2InitManager) InitializeGCV2(_ context.Context, gcLifeTime time.Duration) error { m.initCnt++ m.gcLifeTime = gcLifeTime return nil @@ -220,7 +221,7 @@ func TestInitializeExternalWorkloadGCV2UsesEffectiveGCLifeTime(t *testing.T) { initializeExternalWorkloadGCV2(context.Background(), store, mgr) require.Equal(t, 1, mgr.initCnt) - require.Equal(t, int64(24*60*60), mgr.gcLifeTime) + require.Equal(t, 24*time.Hour, mgr.gcLifeTime) } func TestCreateMgrClientRequiresPodIdentityInStarter(t *testing.T) { diff --git a/pkg/executor/set.go b/pkg/executor/set.go index fc998299f095c..2fd6714d5d910 100644 --- a/pkg/executor/set.go +++ b/pkg/executor/set.go @@ -193,26 +193,7 @@ func (e *SetExecutor) setSysVariable(ctx context.Context, name string, v *expres } logutil.BgLogger().Info(logstr, zap.Uint64("conn", sessionVars.ConnectionID), zap.String("name", name), zap.String("val", showValStr)) if v.IsGlobal && name == vardef.TiDBGCLifetime { - mgr := extworkload.GetManagerFromStore(e.Ctx().GetStore()) - if extworkload.IsMaster(mgr) && pd.IsKeyspaceUsingKeyspaceLevelGC(mgr.Meta()) { - gcLifeTimeVal, err := sessionVars.GlobalVarsAccessor.GetGlobalSysVar(name) - if err != nil { - logutil.BgLogger().Warn("failed to load effective external workload GC life time", - zap.String("name", name), - zap.String("val", showValStr), - zap.Error(err)) - } else if gcLifeTime, err := time.ParseDuration(gcLifeTimeVal); err != nil { - logutil.BgLogger().Warn("failed to parse effective external workload GC life time", - zap.String("name", name), - zap.String("val", gcLifeTimeVal), - zap.Error(err)) - } else if err = mgr.UpdateGCLifeTime(ctx, int64(gcLifeTime/time.Second)); err != nil { - logutil.BgLogger().Warn("failed to update external workload GC life time", - zap.String("name", name), - zap.String("val", gcLifeTimeVal), - zap.Error(err)) - } - } + notifyExternalWorkloadGCLifeTime(ctx, e.Ctx(), showValStr) } if name == vardef.TiDBServiceScope { dom := domain.GetDomain(e.Ctx()) @@ -300,6 +281,36 @@ func (e *SetExecutor) setSysVariable(ctx context.Context, name string, v *expres return nil } +func notifyExternalWorkloadGCLifeTime(ctx context.Context, sctx sessionctx.Context, setValue string) { + mgr := extworkload.GetManagerFromStore(sctx.GetStore()) + if !extworkload.IsEnabled(mgr) || !pd.IsKeyspaceUsingKeyspaceLevelGC(mgr.Meta()) { + return + } + + gcLifeTimeVal, err := variable.GetSysVar(vardef.TiDBGCLifetime).GetGlobalFromHook(ctx, sctx.GetSessionVars()) + if err != nil { + logutil.BgLogger().Warn("failed to load effective external workload GC life time", + zap.String("name", vardef.TiDBGCLifetime), + zap.String("val", setValue), + zap.Error(err)) + return + } + gcLifeTime, err := time.ParseDuration(gcLifeTimeVal) + if err != nil { + logutil.BgLogger().Warn("failed to parse effective external workload GC life time", + zap.String("name", vardef.TiDBGCLifetime), + zap.String("val", gcLifeTimeVal), + zap.Error(err)) + return + } + if err := mgr.UpdateGCLifeTime(ctx, gcLifeTime); err != nil { + logutil.BgLogger().Warn("failed to update external workload GC life time", + zap.String("name", vardef.TiDBGCLifetime), + zap.String("val", gcLifeTimeVal), + zap.Error(err)) + } +} + func (e *SetExecutor) setCharset(cs, co string, isSetName bool) error { var err error sessionVars := e.Ctx().GetSessionVars() diff --git a/pkg/executor/set_test.go b/pkg/executor/set_test.go index 670bc8e541532..79a8b8d446998 100644 --- a/pkg/executor/set_test.go +++ b/pkg/executor/set_test.go @@ -23,6 +23,7 @@ import ( "net/http" "strconv" "testing" + "time" "github.com/pingcap/failpoint" "github.com/pingcap/kvproto/pkg/keyspacepb" @@ -47,39 +48,60 @@ import ( type setGCLifeTimeManager struct { extworkload.Manager - gcLifeTime int64 + gcLifeTime time.Duration updateCnt int + role config.ExternalWorkloadRole + meta *keyspacepb.KeyspaceMeta } func (m *setGCLifeTimeManager) Role() config.ExternalWorkloadRole { - return config.RoleMaster + return m.role } -func (*setGCLifeTimeManager) Meta() *keyspacepb.KeyspaceMeta { - return &keyspacepb.KeyspaceMeta{ - Config: map[string]string{ - pd.KeyspaceConfigGCManagementType: pd.KeyspaceConfigGCManagementTypeKeyspaceLevel, - }, - } +func (m *setGCLifeTimeManager) Meta() *keyspacepb.KeyspaceMeta { + return m.meta } -func (m *setGCLifeTimeManager) UpdateGCLifeTime(_ context.Context, gcLifeTime int64) error { +func (m *setGCLifeTimeManager) UpdateGCLifeTime(_ context.Context, gcLifeTime time.Duration) error { m.updateCnt++ m.gcLifeTime = gcLifeTime return nil } func TestSetGCLifeTimeNotifiesExternalWorkloadWithEffectiveValue(t *testing.T) { - store := testkit.CreateMockStore(t) - mgr := &setGCLifeTimeManager{} - extworkload.SetManagerForStore(store, mgr) - - tk := testkit.NewTestKit(t, store) - tk.MustExec("set global tidb_gc_life_time = '1m'") + keyspaceLevelMeta := &keyspacepb.KeyspaceMeta{Config: map[string]string{ + pd.KeyspaceConfigGCManagementType: pd.KeyspaceConfigGCManagementTypeKeyspaceLevel, + }} + cases := []struct { + name string + role config.ExternalWorkloadRole + meta *keyspacepb.KeyspaceMeta + setValue string + expectedGlobal time.Duration + expectedNotify time.Duration + expectedUpdate int + }{ + {name: "master", role: config.RoleMaster, meta: keyspaceLevelMeta, setValue: "24h", expectedGlobal: 24 * time.Hour, expectedNotify: 24 * time.Hour, expectedUpdate: 1}, + {name: "GCV2 worker", role: config.RoleGCV2Worker, meta: keyspaceLevelMeta, setValue: "24h", expectedGlobal: 24 * time.Hour, expectedNotify: 24 * time.Hour, expectedUpdate: 1}, + {name: "TTL worker", role: config.RoleTTLTaskWorker, meta: keyspaceLevelMeta, setValue: "24h", expectedGlobal: 24 * time.Hour, expectedNotify: 24 * time.Hour, expectedUpdate: 1}, + {name: "auto analyze worker", role: config.RoleAutoAnalyzeWorker, meta: keyspaceLevelMeta, setValue: "24h", expectedGlobal: 24 * time.Hour, expectedNotify: 24 * time.Hour, expectedUpdate: 1}, + {name: "minimum value", role: config.RoleMaster, meta: keyspaceLevelMeta, setValue: "1m", expectedGlobal: 10 * time.Minute, expectedNotify: 10 * time.Minute, expectedUpdate: 1}, + {name: "unified GC", role: config.RoleMaster, meta: &keyspacepb.KeyspaceMeta{}, setValue: "24h", expectedGlobal: 24 * time.Hour, expectedUpdate: 0}, + } - tk.MustQuery("select @@global.tidb_gc_life_time").Check(testkit.Rows("10m0s")) - require.Equal(t, 1, mgr.updateCnt) - require.Equal(t, int64(600), mgr.gcLifeTime) + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + store := testkit.CreateMockStore(t) + mgr := &setGCLifeTimeManager{role: tc.role, meta: tc.meta} + extworkload.SetManagerForStore(store, mgr) + + tk := testkit.NewTestKit(t, store) + tk.MustExec(fmt.Sprintf("set global tidb_gc_life_time = '%s'", tc.setValue)) + tk.MustQuery("select @@global.tidb_gc_life_time").Check(testkit.Rows(tc.expectedGlobal.String())) + require.Equal(t, tc.expectedUpdate, mgr.updateCnt) + require.Equal(t, tc.expectedNotify, mgr.gcLifeTime) + }) + } } func TestSetVar(t *testing.T) { diff --git a/pkg/extworkload/BUILD.bazel b/pkg/extworkload/BUILD.bazel index cbb4e77c10789..406e091a3d1cd 100644 --- a/pkg/extworkload/BUILD.bazel +++ b/pkg/extworkload/BUILD.bazel @@ -14,7 +14,6 @@ go_library( "//pkg/extworkload/client", "//pkg/kv", "//pkg/metrics", - "//pkg/util/intest", "//pkg/util/logutil", "@com_github_pingcap_errors//:errors", "@com_github_pingcap_kvproto//pkg/keyspacepb", @@ -32,7 +31,7 @@ go_test( ], embed = [":extworkload"], flaky = True, - shard_count = 6, + shard_count = 7, deps = [ "//pkg/config", "//pkg/extworkload/client", diff --git a/pkg/extworkload/external_workload.go b/pkg/extworkload/external_workload.go index c3e33de39edba..6261c0201ae8e 100644 --- a/pkg/extworkload/external_workload.go +++ b/pkg/extworkload/external_workload.go @@ -20,6 +20,7 @@ package extworkload import ( "context" + "time" "github.com/pingcap/kvproto/pkg/keyspacepb" "github.com/pingcap/tidb/pkg/config" @@ -35,15 +36,15 @@ type Manager interface { Meta() *keyspacepb.KeyspaceMeta // InitializeGCV2 seeds the controller with an initial keyspace-level GC task. - InitializeGCV2(ctx context.Context, gcLifeTime int64) error + InitializeGCV2(ctx context.Context, gcLifeTime time.Duration) error // AbortGCV2 asks the controller to abort all outstanding keyspace-level GC tasks. AbortGCV2(ctx context.Context) error // RegisterGCV2 reports that a keyspace-level GC round at safePoint has completed. - RegisterGCV2(ctx context.Context, safePoint uint64, gcLifeTime int64) error + RegisterGCV2(ctx context.Context, safePoint uint64, gcLifeTime time.Duration) error // RecycleGCV2 reports that keyspace-level GC up to safePoint has been processed. RecycleGCV2(ctx context.Context, safePoint uint64) error // UpdateGCLifeTime reports that the user's gc_life_time has changed. - UpdateGCLifeTime(ctx context.Context, gcLifeTime int64) error + UpdateGCLifeTime(ctx context.Context, gcLifeTime time.Duration) error // RegisterTTLTask reports that a table with TTL has been created or altered. RegisterTTLTask(ctx context.Context, tableID int64, ttlJobEnable bool) error diff --git a/pkg/extworkload/manager.go b/pkg/extworkload/manager.go index 7fa378d19d810..439804697b2b0 100644 --- a/pkg/extworkload/manager.go +++ b/pkg/extworkload/manager.go @@ -134,11 +134,11 @@ func withRequestTimeout(ctx context.Context) (context.Context, context.CancelFun return context.WithTimeout(ctx, requestTimeout) } -func (m *manager) InitializeGCV2(ctx context.Context, gcLifeTime int64) error { +func (m *manager) InitializeGCV2(ctx context.Context, gcLifeTime time.Duration) error { ctx, cancel := withRequestTimeout(ctx) defer cancel() ctx = withMetric(ctx, string(config.RoleGCV2Worker), metrics.WorkerActionInit) - return m.cli.RegisterGCV2(ctx, 0, gcLifeTime) + return m.cli.RegisterGCV2(ctx, 0, int64(gcLifeTime/time.Second)) } func (m *manager) AbortGCV2(ctx context.Context) error { @@ -149,11 +149,11 @@ func (m *manager) AbortGCV2(ctx context.Context) error { return m.cli.RecycleGCV2(ctx, math.MaxUint64) } -func (m *manager) RegisterGCV2(ctx context.Context, safePoint uint64, gcLifeTime int64) error { +func (m *manager) RegisterGCV2(ctx context.Context, safePoint uint64, gcLifeTime time.Duration) error { ctx, cancel := withRequestTimeout(ctx) defer cancel() ctx = withMetric(ctx, string(config.RoleGCV2Worker), metrics.WorkerActionRegister) - return m.cli.RegisterGCV2(ctx, safePoint, gcLifeTime) + return m.cli.RegisterGCV2(ctx, safePoint, int64(gcLifeTime/time.Second)) } func (m *manager) RecycleGCV2(ctx context.Context, safePoint uint64) error { @@ -163,10 +163,10 @@ func (m *manager) RecycleGCV2(ctx context.Context, safePoint uint64) error { return m.cli.RecycleGCV2(ctx, safePoint) } -func (m *manager) UpdateGCLifeTime(ctx context.Context, gcLifeTime int64) error { +func (m *manager) UpdateGCLifeTime(ctx context.Context, gcLifeTime time.Duration) error { ctx, cancel := withRequestTimeout(ctx) defer cancel() - return m.cli.UpdateGCLifeTime(ctx, gcLifeTime) + return m.cli.UpdateGCLifeTime(ctx, int64(gcLifeTime/time.Second)) } func (m *manager) RegisterTTLTask(ctx context.Context, tableID int64, ttlJobEnable bool) error { diff --git a/pkg/extworkload/manager_test.go b/pkg/extworkload/manager_test.go index 7e2d59fd57180..86476a82d7b61 100644 --- a/pkg/extworkload/manager_test.go +++ b/pkg/extworkload/manager_test.go @@ -20,6 +20,7 @@ import ( "math" "net" "testing" + "time" pb "github.com/pingcap/kvproto/pkg/externalworkloadpb" "github.com/pingcap/kvproto/pkg/keyspacepb" @@ -184,7 +185,7 @@ func TestManagerMethodsSetDeadlineAndMetrics(t *testing.T) { }{ { name: "InitializeGCV2", - call: func(m *manager) error { return m.InitializeGCV2(context.Background(), 3600) }, + call: func(m *manager) error { return m.InitializeGCV2(context.Background(), time.Hour) }, check: func(cli *fakeClient) { require.Equal(t, "RegisterGCV2", cli.call) require.Equal(t, uint64(0), cli.safePoint) @@ -203,7 +204,7 @@ func TestManagerMethodsSetDeadlineAndMetrics(t *testing.T) { }, { name: "RegisterGCV2", - call: func(m *manager) error { return m.RegisterGCV2(context.Background(), 10, 600) }, + call: func(m *manager) error { return m.RegisterGCV2(context.Background(), 10, 10*time.Minute) }, check: func(cli *fakeClient) { require.Equal(t, "RegisterGCV2", cli.call) require.Equal(t, uint64(10), cli.safePoint) @@ -222,7 +223,7 @@ func TestManagerMethodsSetDeadlineAndMetrics(t *testing.T) { }, { name: "UpdateGCLifeTime", - call: func(m *manager) error { return m.UpdateGCLifeTime(context.Background(), 60) }, + call: func(m *manager) error { return m.UpdateGCLifeTime(context.Background(), time.Minute) }, check: func(cli *fakeClient) { require.Equal(t, "UpdateGCLifeTime", cli.call) require.Equal(t, int64(60), cli.gcLifeTime) diff --git a/pkg/extworkload/util.go b/pkg/extworkload/util.go index 37fdcd650c52b..d73a68903f2b0 100644 --- a/pkg/extworkload/util.go +++ b/pkg/extworkload/util.go @@ -16,13 +16,9 @@ package extworkload import ( "context" - "os" "github.com/pingcap/tidb/pkg/config" "github.com/pingcap/tidb/pkg/kv" - "github.com/pingcap/tidb/pkg/util/intest" - "github.com/pingcap/tidb/pkg/util/logutil" - "go.uber.org/zap" ) // IsEnabled reports whether a Manager is present. @@ -63,21 +59,16 @@ func GetManagerFromStore(store kv.Storage) Manager { return mgr } -// AbortGCV2ForUpgrade aborts GCV2 work and exits when this TiDB is the dedicated -// GCV2 worker, which must not run normal bootstrap upgrade work. -func AbortGCV2ForUpgrade(store kv.Storage) { - mgr := GetManagerFromStore(store) +// AbortGCV2ForUpgrade aborts GCV2 work when this TiDB is the dedicated GCV2 +// worker. It returns true when the caller must terminate after the abort. +func AbortGCV2ForUpgrade(ctx context.Context, mgr Manager) (bool, error) { if !IsGCV2Worker(mgr) { - return - } - if err := mgr.AbortGCV2(context.Background()); err != nil { - logutil.BgLogger().Fatal("abort GCV2 worker failed", zap.Error(err)) + return false, nil } - logutil.BgLogger().Info("GCV2 worker aborted") - if intest.InTest { - return + if err := mgr.AbortGCV2(ctx); err != nil { + return false, err } - os.Exit(0) + return true, nil } func roleIs(m Manager, role config.ExternalWorkloadRole) bool { diff --git a/pkg/extworkload/util_test.go b/pkg/extworkload/util_test.go index 643932a9bd59a..27a0b193fd6ba 100644 --- a/pkg/extworkload/util_test.go +++ b/pkg/extworkload/util_test.go @@ -15,6 +15,8 @@ package extworkload import ( + "context" + "errors" "testing" "github.com/pingcap/tidb/pkg/config" @@ -23,11 +25,18 @@ import ( type stubManager struct { Manager - role config.ExternalWorkloadRole + role config.ExternalWorkloadRole + abortCount int + abortErr error } func (s *stubManager) Role() config.ExternalWorkloadRole { return s.role } +func (s *stubManager) AbortGCV2(context.Context) error { + s.abortCount++ + return s.abortErr +} + func TestRolePredicatesWhenDisabled(t *testing.T) { require.False(t, IsEnabled(nil)) require.False(t, IsMaster(nil)) @@ -56,3 +65,30 @@ func TestRolePredicatesDedicated(t *testing.T) { }) } } + +func TestAbortGCV2ForUpgrade(t *testing.T) { + t.Run("other role", func(t *testing.T) { + mgr := &stubManager{role: config.RoleMaster} + shouldTerminate, err := AbortGCV2ForUpgrade(context.Background(), mgr) + require.NoError(t, err) + require.False(t, shouldTerminate) + require.Zero(t, mgr.abortCount) + }) + + t.Run("GCV2 worker", func(t *testing.T) { + mgr := &stubManager{role: config.RoleGCV2Worker} + shouldTerminate, err := AbortGCV2ForUpgrade(context.Background(), mgr) + require.NoError(t, err) + require.True(t, shouldTerminate) + require.Equal(t, 1, mgr.abortCount) + }) + + t.Run("abort error", func(t *testing.T) { + abortErr := errors.New("abort failed") + mgr := &stubManager{role: config.RoleGCV2Worker, abortErr: abortErr} + shouldTerminate, err := AbortGCV2ForUpgrade(context.Background(), mgr) + require.ErrorIs(t, err, abortErr) + require.False(t, shouldTerminate) + require.Equal(t, 1, mgr.abortCount) + }) +} diff --git a/pkg/session/session.go b/pkg/session/session.go index bfb9091388c48..9c2c2089f0958 100644 --- a/pkg/session/session.go +++ b/pkg/session/session.go @@ -30,6 +30,7 @@ import ( "maps" "math" "math/rand" + "os" "regexp" "runtime/pprof" "slices" @@ -4598,7 +4599,6 @@ func runInBootstrapSession(store kv.Storage, ver int64) { zap.Duration("cost", time.Since(startTime))) }() if startMode == ddl.Upgrade { - extworkload.AbortGCV2ForUpgrade(store) // TODO at this time domain must not be created, else it will register server // info, and cause deadlock, we need to make sure this in a clear way logutil.BgLogger().Info("[upgrade] get owner lock to upgrade") @@ -4616,6 +4616,19 @@ func runInBootstrapSession(store kv.Storage, ver int64) { // TODO remove this after we can refactor below code out in this case. logutil.BgLogger().Info("[upgrade] already upgraded by other nodes, switch to normal mode") startMode = ddl.Normal + } else if deploymode.IsStarter() { + shouldTerminate, err := extworkload.AbortGCV2ForUpgrade(context.Background(), extworkload.GetManagerFromStore(store)) + if err != nil { + logutil.BgLogger().Fatal("abort GCV2 worker failed", zap.Error(err)) + } + if shouldTerminate { + logutil.BgLogger().Info("GCV2 worker aborted before bootstrap upgrade") + if intest.InTest { + return + } + releaseFn() + os.Exit(0) + } } } s, err := createSession(store) diff --git a/pkg/session/session_nextgen_test.go b/pkg/session/session_nextgen_test.go index 9b1d9f8384cb9..5a5beb439b596 100644 --- a/pkg/session/session_nextgen_test.go +++ b/pkg/session/session_nextgen_test.go @@ -20,11 +20,27 @@ import ( "context" "testing" + "github.com/pingcap/tidb/pkg/config" "github.com/pingcap/tidb/pkg/config/deploymode" + "github.com/pingcap/tidb/pkg/extworkload" "github.com/pingcap/tidb/pkg/sessionctx/variable" "github.com/stretchr/testify/require" ) +type upgradeGCV2Manager struct { + extworkload.Manager + abortCount int +} + +func (*upgradeGCV2Manager) Role() config.ExternalWorkloadRole { + return config.RoleGCV2Worker +} + +func (m *upgradeGCV2Manager) AbortGCV2(context.Context) error { + m.abortCount++ + return nil +} + func TestUsePipelinedDMLDisabledInStarter(t *testing.T) { originalMode := deploymode.Get() require.NoError(t, deploymode.Set(deploymode.Starter)) @@ -41,3 +57,24 @@ func TestUsePipelinedDMLDisabledInStarter(t *testing.T) { require.Len(t, warnings, 1) require.EqualError(t, warnings[0].Err, "Pipelined DML is not supported in this deployment. Fallback to standard mode") } + +func TestUpgradeGCV2AbortUsesPostLockBootstrapVersion(t *testing.T) { + originalMode := deploymode.Get() + require.NoError(t, deploymode.Set(deploymode.Starter)) + t.Cleanup(func() { + require.NoError(t, deploymode.Set(originalMode)) + }) + + store, dom := CreateStoreAndBootstrap(t) + t.Cleanup(func() { + require.NoError(t, store.Close()) + }) + dom.Close() + domap.Delete(store) + + mgr := &upgradeGCV2Manager{} + extworkload.SetManagerForStore(store, mgr) + runInBootstrapSession(store, currentBootstrapVersion-1) + + require.Zero(t, mgr.abortCount) +} diff --git a/pkg/session/upgrade_run.go b/pkg/session/upgrade_run.go index e170aace006b6..28db74b0637fc 100644 --- a/pkg/session/upgrade_run.go +++ b/pkg/session/upgrade_run.go @@ -18,7 +18,6 @@ import ( "context" "time" - "github.com/pingcap/tidb/pkg/extworkload" "github.com/pingcap/tidb/pkg/kv" "github.com/pingcap/tidb/pkg/meta" "github.com/pingcap/tidb/pkg/parser/terror" @@ -45,8 +44,6 @@ func upgrade(s sessionapi.Session) { return } - extworkload.AbortGCV2ForUpgrade(s.GetStore()) - printClusterState(s, ver) // when upgrade from v6.4.0 or earlier, enables metadata lock automatically, diff --git a/pkg/store/gcworker/gc_worker.go b/pkg/store/gcworker/gc_worker.go index ad8eb26bf2899..f38b5a7c0ac46 100644 --- a/pkg/store/gcworker/gc_worker.go +++ b/pkg/store/gcworker/gc_worker.go @@ -866,7 +866,7 @@ func (w *GCWorker) notifyGCV2AfterGC(ctx context.Context, safePoint uint64) { logutil.Logger(ctx).Warn("failed to load GC life time for external workload", zap.String("category", "gc worker"), zap.Error(err)) - } else if err := mgr.RegisterGCV2(ctx, safePoint, int64(*gcLifeTime/time.Second)); err != nil { + } else if err := mgr.RegisterGCV2(ctx, safePoint, *gcLifeTime); err != nil { logutil.Logger(ctx).Warn("failed to register GCV2 task", zap.String("category", "gc worker"), zap.Uint64("safePoint", safePoint), diff --git a/pkg/store/gcworker/gc_worker_test.go b/pkg/store/gcworker/gc_worker_test.go index 57e352bbf20ee..937cd3c98a06e 100644 --- a/pkg/store/gcworker/gc_worker_test.go +++ b/pkg/store/gcworker/gc_worker_test.go @@ -83,7 +83,7 @@ type stubGCV2Manager struct { func (m *stubGCV2Manager) Role() config.ExternalWorkloadRole { return m.role } func (m *stubGCV2Manager) Meta() *keyspacepb.KeyspaceMeta { return m.meta } -func (m *stubGCV2Manager) RegisterGCV2(_ context.Context, _ uint64, _ int64) error { +func (m *stubGCV2Manager) RegisterGCV2(_ context.Context, _ uint64, _ time.Duration) error { m.registerCount++ return m.err } From c9f91f82ed60f0c05a97a07983795538373ea039 Mon Sep 17 00:00:00 2001 From: AmoebaProtozoa <8039876+AmoebaProtozoa@users.noreply.github.com> Date: Wed, 15 Jul 2026 02:35:44 +0800 Subject: [PATCH 28/28] external workload: align GCV2 conversions and exit logging --- pkg/extworkload/manager.go | 6 +++--- pkg/session/session.go | 4 +--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/pkg/extworkload/manager.go b/pkg/extworkload/manager.go index 439804697b2b0..0bd1be0e6de72 100644 --- a/pkg/extworkload/manager.go +++ b/pkg/extworkload/manager.go @@ -138,7 +138,7 @@ func (m *manager) InitializeGCV2(ctx context.Context, gcLifeTime time.Duration) ctx, cancel := withRequestTimeout(ctx) defer cancel() ctx = withMetric(ctx, string(config.RoleGCV2Worker), metrics.WorkerActionInit) - return m.cli.RegisterGCV2(ctx, 0, int64(gcLifeTime/time.Second)) + return m.cli.RegisterGCV2(ctx, 0, int64(gcLifeTime.Seconds())) } func (m *manager) AbortGCV2(ctx context.Context) error { @@ -153,7 +153,7 @@ func (m *manager) RegisterGCV2(ctx context.Context, safePoint uint64, gcLifeTime ctx, cancel := withRequestTimeout(ctx) defer cancel() ctx = withMetric(ctx, string(config.RoleGCV2Worker), metrics.WorkerActionRegister) - return m.cli.RegisterGCV2(ctx, safePoint, int64(gcLifeTime/time.Second)) + return m.cli.RegisterGCV2(ctx, safePoint, int64(gcLifeTime.Seconds())) } func (m *manager) RecycleGCV2(ctx context.Context, safePoint uint64) error { @@ -166,7 +166,7 @@ func (m *manager) RecycleGCV2(ctx context.Context, safePoint uint64) error { func (m *manager) UpdateGCLifeTime(ctx context.Context, gcLifeTime time.Duration) error { ctx, cancel := withRequestTimeout(ctx) defer cancel() - return m.cli.UpdateGCLifeTime(ctx, int64(gcLifeTime/time.Second)) + return m.cli.UpdateGCLifeTime(ctx, int64(gcLifeTime.Seconds())) } func (m *manager) RegisterTTLTask(ctx context.Context, tableID int64, ttlJobEnable bool) error { diff --git a/pkg/session/session.go b/pkg/session/session.go index 9c2c2089f0958..bc8e956137010 100644 --- a/pkg/session/session.go +++ b/pkg/session/session.go @@ -30,7 +30,6 @@ import ( "maps" "math" "math/rand" - "os" "regexp" "runtime/pprof" "slices" @@ -4622,12 +4621,11 @@ func runInBootstrapSession(store kv.Storage, ver int64) { logutil.BgLogger().Fatal("abort GCV2 worker failed", zap.Error(err)) } if shouldTerminate { - logutil.BgLogger().Info("GCV2 worker aborted before bootstrap upgrade") if intest.InTest { return } releaseFn() - os.Exit(0) + logutil.BgLogger().Fatal("GCV2 worker aborted before bootstrap upgrade") } } }