From 1711424a8133fec580f47067f40f765e6cd7ee7e Mon Sep 17 00:00:00 2001 From: ystaticy Date: Mon, 16 Jan 2023 23:13:17 +0800 Subject: [PATCH 01/18] add keyspace drop table logic Signed-off-by: ystaticy --- ddl/attributes_sql_test.go | 12 +++--- ddl/main_test.go | 2 +- domain/domain.go | 51 +++++++++++++--------- domain/infosync/info.go | 32 +++++++------- store/driver/tikv_driver.go | 4 -- store/gcworker/gc_worker.go | 84 +++++++++++++++++++++++++++++++++++++ 6 files changed, 139 insertions(+), 46 deletions(-) diff --git a/ddl/attributes_sql_test.go b/ddl/attributes_sql_test.go index 95f881e6fb3fe..2d2685ffd06b2 100644 --- a/ddl/attributes_sql_test.go +++ b/ddl/attributes_sql_test.go @@ -269,7 +269,7 @@ PARTITION BY RANGE (c) ( func TestFlashbackTable(t *testing.T) { store, dom := testkit.CreateMockStoreAndDomain(t) - _, err := infosync.GlobalInfoSyncerInit(context.Background(), dom.DDL().GetID(), dom.ServerID, dom.GetEtcdClient(), true) + _, err := infosync.GlobalInfoSyncerInit(context.Background(), dom.DDL().GetID(), dom.ServerID, dom.GetEtcdClient(), dom.GetEtcdClient(), true) require.NoError(t, err) tk := testkit.NewTestKit(t, store) tk.MustExec("use test") @@ -327,7 +327,7 @@ PARTITION BY RANGE (c) ( func TestDropTable(t *testing.T) { store, dom := testkit.CreateMockStoreAndDomain(t) - _, err := infosync.GlobalInfoSyncerInit(context.Background(), dom.DDL().GetID(), dom.ServerID, dom.GetEtcdClient(), true) + _, err := infosync.GlobalInfoSyncerInit(context.Background(), dom.DDL().GetID(), dom.ServerID, dom.GetEtcdClient(), dom.GetEtcdClient(), true) require.NoError(t, err) tk := testkit.NewTestKit(t, store) tk.MustExec("use test") @@ -380,7 +380,7 @@ PARTITION BY RANGE (c) ( func TestCreateWithSameName(t *testing.T) { store, dom := testkit.CreateMockStoreAndDomain(t) - _, err := infosync.GlobalInfoSyncerInit(context.Background(), dom.DDL().GetID(), dom.ServerID, dom.GetEtcdClient(), true) + _, err := infosync.GlobalInfoSyncerInit(context.Background(), dom.DDL().GetID(), dom.ServerID, dom.GetEtcdClient(), dom.GetEtcdClient(), true) require.NoError(t, err) tk := testkit.NewTestKit(t, store) tk.MustExec("use test") @@ -444,7 +444,7 @@ PARTITION BY RANGE (c) ( func TestPartition(t *testing.T) { store, dom := testkit.CreateMockStoreAndDomain(t) - _, err := infosync.GlobalInfoSyncerInit(context.Background(), dom.DDL().GetID(), dom.ServerID, dom.GetEtcdClient(), true) + _, err := infosync.GlobalInfoSyncerInit(context.Background(), dom.DDL().GetID(), dom.ServerID, dom.GetEtcdClient(), dom.GetEtcdClient(), true) require.NoError(t, err) tk := testkit.NewTestKit(t, store) tk.MustExec("use test") @@ -504,7 +504,7 @@ PARTITION BY RANGE (c) ( func TestDropSchema(t *testing.T) { store, dom := testkit.CreateMockStoreAndDomain(t) - _, err := infosync.GlobalInfoSyncerInit(context.Background(), dom.DDL().GetID(), dom.ServerID, dom.GetEtcdClient(), true) + _, err := infosync.GlobalInfoSyncerInit(context.Background(), dom.DDL().GetID(), dom.ServerID, dom.GetEtcdClient(), dom.GetEtcdClient(), true) require.NoError(t, err) tk := testkit.NewTestKit(t, store) tk.MustExec("use test") @@ -530,7 +530,7 @@ PARTITION BY RANGE (c) ( func TestDefaultKeyword(t *testing.T) { store, dom := testkit.CreateMockStoreAndDomain(t) - _, err := infosync.GlobalInfoSyncerInit(context.Background(), dom.DDL().GetID(), dom.ServerID, dom.GetEtcdClient(), true) + _, err := infosync.GlobalInfoSyncerInit(context.Background(), dom.DDL().GetID(), dom.ServerID, dom.GetEtcdClient(), dom.GetEtcdClient(), true) require.NoError(t, err) tk := testkit.NewTestKit(t, store) tk.MustExec("use test") diff --git a/ddl/main_test.go b/ddl/main_test.go index 6a8642ae34380..a10374b04f0f2 100644 --- a/ddl/main_test.go +++ b/ddl/main_test.go @@ -52,7 +52,7 @@ func TestMain(m *testing.M) { conf.Experimental.AllowsExpressionIndex = true }) - _, err := infosync.GlobalInfoSyncerInit(context.Background(), "t", func() uint64 { return 1 }, nil, true) + _, err := infosync.GlobalInfoSyncerInit(context.Background(), "t", func() uint64 { return 1 }, nil, nil, true) if err != nil { _, _ = fmt.Fprintf(os.Stderr, "ddl: infosync.GlobalInfoSyncerInit: %v\n", err) os.Exit(1) diff --git a/domain/domain.go b/domain/domain.go index 977694d773998..41e64d50f849e 100644 --- a/domain/domain.go +++ b/domain/domain.go @@ -111,6 +111,7 @@ type Domain struct { sysSessionPool *sessionPool exit chan struct{} etcdClient *clientv3.Client + unprefixedEtcdCli *clientv3.Client sysVarCache sysVarCache // replaces GlobalVariableCache slowQuery *topNSlowQueries expensiveQueryHandle *expensivequery.Handle @@ -930,6 +931,27 @@ func NewDomain(store kv.Storage, ddlLease time.Duration, statsLease time.Duratio const serverIDForStandalone = 1 // serverID for standalone deployment. +func newEtcdCli(addrs []string, ebd kv.EtcdBackend) (*clientv3.Client, error) { + cfg := config.GetGlobalConfig() + etcdLogCfg := zap.NewProductionConfig() + etcdLogCfg.Level = zap.NewAtomicLevelAt(zap.ErrorLevel) + cli, err := clientv3.New(clientv3.Config{ + LogConfig: &etcdLogCfg, + Endpoints: addrs, + AutoSyncInterval: 30 * time.Second, + DialTimeout: 5 * time.Second, + DialOptions: []grpc.DialOption{ + grpc.WithBackoffMaxDelay(time.Second * 3), + grpc.WithKeepaliveParams(keepalive.ClientParameters{ + Time: time.Duration(cfg.TiKVClient.GrpcKeepAliveTime) * time.Second, + Timeout: time.Duration(cfg.TiKVClient.GrpcKeepAliveTimeout) * time.Second, + }), + }, + TLS: ebd.TLSConfig(), + }) + return cli, err +} + // Init initializes a domain. func (do *Domain) Init( ddlLease time.Duration, @@ -945,25 +967,8 @@ func (do *Domain) Init( return err } if addrs != nil { - cfg := config.GetGlobalConfig() - // silence etcd warn log, when domain closed, it won't randomly print warn log - // see details at the issue https://github.com/pingcap/tidb/issues/15479 - etcdLogCfg := zap.NewProductionConfig() - etcdLogCfg.Level = zap.NewAtomicLevelAt(zap.ErrorLevel) - cli, err := clientv3.New(clientv3.Config{ - LogConfig: &etcdLogCfg, - Endpoints: addrs, - AutoSyncInterval: 30 * time.Second, - DialTimeout: 5 * time.Second, - DialOptions: []grpc.DialOption{ - grpc.WithBackoffMaxDelay(time.Second * 3), - grpc.WithKeepaliveParams(keepalive.ClientParameters{ - Time: time.Duration(cfg.TiKVClient.GrpcKeepAliveTime) * time.Second, - Timeout: time.Duration(cfg.TiKVClient.GrpcKeepAliveTimeout) * time.Second, - }), - }, - TLS: ebd.TLSConfig(), - }) + + cli, err := newEtcdCli(addrs, ebd) if err != nil { return errors.Trace(err) } @@ -971,6 +976,12 @@ func (do *Domain) Init( etcd.SetEtcdCliByNamespace(cli, keyspace.MakeKeyspaceEtcdNamespace(do.store.GetCodec())) do.etcdClient = cli + + noNamespaceCli, err := newEtcdCli(addrs, ebd) + if err != nil { + return errors.Trace(err) + } + do.unprefixedEtcdCli = noNamespaceCli } } @@ -1032,7 +1043,7 @@ func (do *Domain) Init( // step 1: prepare the info/schema syncer which domain reload needed. skipRegisterToDashboard := config.GetGlobalConfig().SkipRegisterToDashboard - do.info, err = infosync.GlobalInfoSyncerInit(ctx, do.ddl.GetID(), do.ServerID, do.etcdClient, skipRegisterToDashboard) + do.info, err = infosync.GlobalInfoSyncerInit(ctx, do.ddl.GetID(), do.ServerID, do.etcdClient, do.unprefixedEtcdCli, skipRegisterToDashboard) if err != nil { return err } diff --git a/domain/infosync/info.go b/domain/infosync/info.go index 7732a831b057e..a740adf3b08e4 100644 --- a/domain/infosync/info.go +++ b/domain/infosync/info.go @@ -95,12 +95,13 @@ var ErrPrometheusAddrIsNotSet = dbterror.ClassDomain.NewStd(errno.ErrPrometheusA // InfoSyncer stores server info to etcd when the tidb-server starts and delete when tidb-server shuts down. type InfoSyncer struct { - etcdCli *clientv3.Client - info *ServerInfo - serverInfoPath string - minStartTS uint64 - minStartTSPath string - managerMu struct { + etcdCli *clientv3.Client + unprefixedEtcdCli *clientv3.Client + info *ServerInfo + serverInfoPath string + minStartTS uint64 + minStartTSPath string + managerMu struct { mu sync.RWMutex util2.SessionManager } @@ -180,12 +181,13 @@ func setGlobalInfoSyncer(is *InfoSyncer) { } // GlobalInfoSyncerInit return a new InfoSyncer. It is exported for testing. -func GlobalInfoSyncerInit(ctx context.Context, id string, serverIDGetter func() uint64, etcdCli *clientv3.Client, skipRegisterToDashBoard bool) (*InfoSyncer, error) { +func GlobalInfoSyncerInit(ctx context.Context, id string, serverIDGetter func() uint64, etcdCli *clientv3.Client, unprefixedEtcdCli *clientv3.Client, skipRegisterToDashBoard bool) (*InfoSyncer, error) { is := &InfoSyncer{ - etcdCli: etcdCli, - info: getServerInfo(id, serverIDGetter), - serverInfoPath: fmt.Sprintf("%s/%s", ServerInformationPath, id), - minStartTSPath: fmt.Sprintf("%s/%s", ServerMinStartTSPath, id), + etcdCli: etcdCli, + unprefixedEtcdCli: unprefixedEtcdCli, + info: getServerInfo(id, serverIDGetter), + serverInfoPath: fmt.Sprintf("%s/%s", ServerInformationPath, id), + minStartTSPath: fmt.Sprintf("%s/%s", ServerMinStartTSPath, id), } err := is.init(ctx, skipRegisterToDashBoard) if err != nil { @@ -640,7 +642,7 @@ func (is *InfoSyncer) getAllServerInfo(ctx context.Context) (map[string]*ServerI // StoreServerInfo stores self server static information to etcd. func (is *InfoSyncer) StoreServerInfo(ctx context.Context) error { - if is.etcdCli == nil { + if is.unprefixedEtcdCli == nil { return nil } infoBuf, err := is.info.Marshal() @@ -648,7 +650,7 @@ func (is *InfoSyncer) StoreServerInfo(ctx context.Context) error { return errors.Trace(err) } str := string(hack.String(infoBuf)) - err = util.PutKVToEtcd(ctx, is.etcdCli, keyOpDefaultRetryCnt, is.serverInfoPath, str, clientv3.WithLease(is.session.Lease())) + err = util.PutKVToEtcd(ctx, is.unprefixedEtcdCli, keyOpDefaultRetryCnt, is.serverInfoPath, str, clientv3.WithLease(is.session.Lease())) return err } @@ -731,10 +733,10 @@ func (is *InfoSyncer) storeMinStartTS(ctx context.Context) error { // RemoveMinStartTS removes self server min start timestamp from etcd. func (is *InfoSyncer) RemoveMinStartTS() { - if is.etcdCli == nil { + if is.unprefixedEtcdCli == nil { return } - err := util.DeleteKeyFromEtcd(is.minStartTSPath, is.etcdCli, keyOpDefaultRetryCnt, keyOpDefaultTimeout) + err := util.DeleteKeyFromEtcd(is.minStartTSPath, is.unprefixedEtcdCli, keyOpDefaultRetryCnt, keyOpDefaultTimeout) if err != nil { logutil.BgLogger().Error("remove minStartTS failed", zap.Error(err)) } diff --git a/store/driver/tikv_driver.go b/store/driver/tikv_driver.go index bf0f1272184dd..ead64d68b35ac 100644 --- a/store/driver/tikv_driver.go +++ b/store/driver/tikv_driver.go @@ -172,10 +172,6 @@ func (d TiKVDriver) OpenWithOptions(path string, options ...Option) (kv.Storage, if err != nil { return nil, errors.Trace(err) } - // If there's setting keyspace-name, then skipped GC worker logic. - // It needs a group of special tidb nodes to execute GC worker logic. - // TODO: remove this restriction while merged keyspace GC worker logic. - disableGC = true } codec := pdClient.GetCodec() diff --git a/store/gcworker/gc_worker.go b/store/gcworker/gc_worker.go index 87a990f7f096c..e92114d38684e 100644 --- a/store/gcworker/gc_worker.go +++ b/store/gcworker/gc_worker.go @@ -301,6 +301,58 @@ func (w *GCWorker) tick(ctx context.Context) { } } +// 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, + // it will return current gc safepoint. + safePoint, err := pdClient.UpdateGCSafePoint(ctx, 0) + if err != nil { + return 0, errors.Trace(err) + } + return safePoint, nil +} + +func (w *GCWorker) runKeyspaceDeleteRange(ctx context.Context, concurrency int) error { + + // Get safepoint from PD. + safePoint, err := getGCSafePoint(ctx, w.pdClient) + + keyspaceID := w.store.GetCodec().GetKeyspaceID() + logutil.Logger(ctx).Info("[gc worker] start keyspace delete range", + zap.String("uuid", w.uuid), + zap.Int("concurrency", concurrency), + zap.Uint32("keyspaceID", uint32(keyspaceID)), + zap.Uint64("GCSafepoint", safePoint)) + + if safePoint == 0 { + logutil.Logger(ctx).Info("[gc worker] skip keyspace delete range, because gc safepoint is 0") + return nil + } + + // Do deleteRanges. + err = w.deleteRanges(ctx, safePoint, concurrency) + if err != nil { + logutil.Logger(ctx).Error("[gc worker] delete range returns an error", + zap.String("uuid", w.uuid), + zap.Error(err)) + metrics.GCJobFailureCounter.WithLabelValues("delete_range").Inc() + return errors.Trace(err) + } + + // Do redoDeleteRanges. + err = w.redoDeleteRanges(ctx, safePoint, concurrency) + if err != nil { + logutil.Logger(ctx).Error("[gc worker] redo-delete range returns an error", + zap.String("uuid", w.uuid), + zap.Error(err)) + metrics.GCJobFailureCounter.WithLabelValues("redo_delete_range").Inc() + return errors.Trace(err) + } + + return nil +} + // leaderTick of GC worker checks if it should start a GC job every tick. func (w *GCWorker) leaderTick(ctx context.Context) error { if w.gcIsRunning { @@ -317,6 +369,38 @@ func (w *GCWorker) leaderTick(ctx context.Context) error { return errors.Trace(err) } + // Do keyspace delete range + if w.store.GetCodec().GetKeyspace() == nil { + + // When the worker is just started, or an old GC job has just finished, + // wait a while before starting a new job. + if time.Since(w.lastFinish) < gcWaitTime { + logutil.Logger(ctx).Info("[gc worker] another keyspace gc job has just finished, skipped.", + zap.String("leaderTick on ", w.uuid)) + return nil + } + + now, err := w.getOracleTime() + if err != nil { + return errors.Trace(err) + } + ok, err := w.checkGCInterval(now) + if err != nil || !ok { + return errors.Trace(err) + } + + go func() { + w.done <- w.runKeyspaceDeleteRange(ctx, concurrency) + }() + + err = w.saveTime(gcLastRunTimeKey, now) + if err != nil { + return errors.Trace(err) + } + + return nil + } + ok, safePoint, err := w.prepare(ctx) if err != nil { metrics.GCJobFailureCounter.WithLabelValues("prepare").Inc() From 3752de0755daad38adebe7544b68bc40ecf1cc23 Mon Sep 17 00:00:00 2001 From: ystaticy Date: Tue, 17 Jan 2023 00:16:48 +0800 Subject: [PATCH 02/18] add keyspace drop table logic Signed-off-by: ystaticy --- domain/db_test.go | 4 ++-- domain/domain.go | 1 - domain/infosync/info_test.go | 6 +++--- server/stat_test.go | 2 +- store/gcworker/gc_worker.go | 12 ++++++------ 5 files changed, 12 insertions(+), 13 deletions(-) diff --git a/domain/db_test.go b/domain/db_test.go index ff6e6b625788a..9b122664f8397 100644 --- a/domain/db_test.go +++ b/domain/db_test.go @@ -73,7 +73,7 @@ func TestNormalSessionPool(t *testing.T) { domain, err := session.BootstrapSession(store) require.NoError(t, err) defer domain.Close() - info, err1 := infosync.GlobalInfoSyncerInit(context.Background(), "t", func() uint64 { return 1 }, nil, true) + info, err1 := infosync.GlobalInfoSyncerInit(context.Background(), "t", func() uint64 { return 1 }, nil, nil, true) require.NoError(t, err1) conf := config.GetGlobalConfig() conf.Socket = "" @@ -107,7 +107,7 @@ func TestAbnormalSessionPool(t *testing.T) { domain, err := session.BootstrapSession(store) require.NoError(t, err) defer domain.Close() - info, err1 := infosync.GlobalInfoSyncerInit(context.Background(), "t", func() uint64 { return 1 }, nil, true) + info, err1 := infosync.GlobalInfoSyncerInit(context.Background(), "t", func() uint64 { return 1 }, nil, nil, true) require.NoError(t, err1) conf := config.GetGlobalConfig() conf.Socket = "" diff --git a/domain/domain.go b/domain/domain.go index 41e64d50f849e..b67ffef7df133 100644 --- a/domain/domain.go +++ b/domain/domain.go @@ -967,7 +967,6 @@ func (do *Domain) Init( return err } if addrs != nil { - cli, err := newEtcdCli(addrs, ebd) if err != nil { return errors.Trace(err) diff --git a/domain/infosync/info_test.go b/domain/infosync/info_test.go index 90a30d8f1f161..3264c0adca3c6 100644 --- a/domain/infosync/info_test.go +++ b/domain/infosync/info_test.go @@ -67,7 +67,7 @@ func TestTopology(t *testing.T) { require.NoError(t, err) }() - info, err := GlobalInfoSyncerInit(ctx, currentID, func() uint64 { return 1 }, client, false) + info, err := GlobalInfoSyncerInit(ctx, currentID, func() uint64 { return 1 }, client, client, false) require.NoError(t, err) err = info.newTopologySessionAndStoreServerInfo(ctx, util2.NewSessionDefaultRetryCnt) @@ -152,7 +152,7 @@ func (is *InfoSyncer) ttlKeyExists(ctx context.Context) (bool, error) { } func TestPutBundlesRetry(t *testing.T) { - _, err := GlobalInfoSyncerInit(context.TODO(), "test", func() uint64 { return 1 }, nil, false) + _, err := GlobalInfoSyncerInit(context.TODO(), "test", func() uint64 { return 1 }, nil, nil, false) require.NoError(t, err) bundle, err := placement.NewBundleFromOptions(&model.PlacementSettings{PrimaryRegion: "r1", Regions: "r1,r2"}) @@ -216,7 +216,7 @@ func TestPutBundlesRetry(t *testing.T) { func TestTiFlashManager(t *testing.T) { ctx := context.Background() - _, err := GlobalInfoSyncerInit(ctx, "test", func() uint64 { return 1 }, nil, false) + _, err := GlobalInfoSyncerInit(ctx, "test", func() uint64 { return 1 }, nil, nil, false) tiflash := NewMockTiFlash() SetMockTiFlash(tiflash) diff --git a/server/stat_test.go b/server/stat_test.go index 66c974a3deeea..4484823f6dc83 100644 --- a/server/stat_test.go +++ b/server/stat_test.go @@ -46,7 +46,7 @@ func TestUptime(t *testing.T) { }() require.NoError(t, err) - _, err = infosync.GlobalInfoSyncerInit(context.Background(), dom.DDL().GetID(), dom.ServerID, dom.GetEtcdClient(), true) + _, err = infosync.GlobalInfoSyncerInit(context.Background(), dom.DDL().GetID(), dom.ServerID, dom.GetEtcdClient(), dom.GetEtcdClient(), true) require.NoError(t, err) tidbdrv := NewTiDBDriver(store) diff --git a/store/gcworker/gc_worker.go b/store/gcworker/gc_worker.go index e92114d38684e..8c090946513c7 100644 --- a/store/gcworker/gc_worker.go +++ b/store/gcworker/gc_worker.go @@ -303,7 +303,6 @@ func (w *GCWorker) tick(ctx context.Context) { // 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, // it will return current gc safepoint. safePoint, err := pdClient.UpdateGCSafePoint(ctx, 0) @@ -314,10 +313,12 @@ func getGCSafePoint(ctx context.Context, pdClient pd.Client) (uint64, error) { } func (w *GCWorker) runKeyspaceDeleteRange(ctx context.Context, concurrency int) error { - - // Get safepoint from PD. + // Get safe point from PD. safePoint, err := getGCSafePoint(ctx, w.pdClient) - + if safePoint == 0 { + logutil.Logger(ctx).Info("[gc worker] get gc safe point error", zap.Error(errors.Trace(err))) + return nil + } keyspaceID := w.store.GetCodec().GetKeyspaceID() logutil.Logger(ctx).Info("[gc worker] start keyspace delete range", zap.String("uuid", w.uuid), @@ -326,7 +327,7 @@ func (w *GCWorker) runKeyspaceDeleteRange(ctx context.Context, concurrency int) zap.Uint64("GCSafepoint", safePoint)) if safePoint == 0 { - logutil.Logger(ctx).Info("[gc worker] skip keyspace delete range, because gc safepoint is 0") + logutil.Logger(ctx).Info("[gc worker] skip keyspace delete range, because gc safe point is 0") return nil } @@ -371,7 +372,6 @@ func (w *GCWorker) leaderTick(ctx context.Context) error { // Do keyspace delete range if w.store.GetCodec().GetKeyspace() == nil { - // When the worker is just started, or an old GC job has just finished, // wait a while before starting a new job. if time.Since(w.lastFinish) < gcWaitTime { From 9534706b662a9ed4e2a18a9d89e1ca1c6fc677a0 Mon Sep 17 00:00:00 2001 From: ystaticy Date: Sat, 28 Jan 2023 18:21:09 +0800 Subject: [PATCH 03/18] fix condition of keyspace Signed-off-by: ystaticy --- store/gcworker/gc_worker.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/store/gcworker/gc_worker.go b/store/gcworker/gc_worker.go index 8c090946513c7..76d4c46beb940 100644 --- a/store/gcworker/gc_worker.go +++ b/store/gcworker/gc_worker.go @@ -371,7 +371,7 @@ func (w *GCWorker) leaderTick(ctx context.Context) error { } // Do keyspace delete range - if w.store.GetCodec().GetKeyspace() == nil { + if w.store.GetCodec().GetKeyspace() != nil { // When the worker is just started, or an old GC job has just finished, // wait a while before starting a new job. if time.Since(w.lastFinish) < gcWaitTime { From 7fac6846cf6e6e22fd5803f7d1c42f6e6a21e13b Mon Sep 17 00:00:00 2001 From: ystaticy Date: Sun, 29 Jan 2023 10:55:26 +0800 Subject: [PATCH 04/18] fix etcdcli in ServerInformationPath Signed-off-by: ystaticy --- domain/infosync/info.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/domain/infosync/info.go b/domain/infosync/info.go index a740adf3b08e4..f6dbecca0817c 100644 --- a/domain/infosync/info.go +++ b/domain/infosync/info.go @@ -642,7 +642,7 @@ func (is *InfoSyncer) getAllServerInfo(ctx context.Context) (map[string]*ServerI // StoreServerInfo stores self server static information to etcd. func (is *InfoSyncer) StoreServerInfo(ctx context.Context) error { - if is.unprefixedEtcdCli == nil { + if is.etcdCli == nil { return nil } infoBuf, err := is.info.Marshal() @@ -650,7 +650,7 @@ func (is *InfoSyncer) StoreServerInfo(ctx context.Context) error { return errors.Trace(err) } str := string(hack.String(infoBuf)) - err = util.PutKVToEtcd(ctx, is.unprefixedEtcdCli, keyOpDefaultRetryCnt, is.serverInfoPath, str, clientv3.WithLease(is.session.Lease())) + err = util.PutKVToEtcd(ctx, is.etcdCli, keyOpDefaultRetryCnt, is.serverInfoPath, str, clientv3.WithLease(is.session.Lease())) return err } @@ -723,10 +723,10 @@ func (is *InfoSyncer) GetMinStartTS() uint64 { // storeMinStartTS stores self server min start timestamp to etcd. func (is *InfoSyncer) storeMinStartTS(ctx context.Context) error { - if is.etcdCli == nil { + if is.unprefixedEtcdCli == nil { return nil } - return util.PutKVToEtcd(ctx, is.etcdCli, keyOpDefaultRetryCnt, is.minStartTSPath, + return util.PutKVToEtcd(ctx, is.unprefixedEtcdCli, keyOpDefaultRetryCnt, is.minStartTSPath, strconv.FormatUint(is.minStartTS, 10), clientv3.WithLease(is.session.Lease())) } From 168a9b2e29da43fdf5ebf2d6ef353fa546d3904d Mon Sep 17 00:00:00 2001 From: ystaticy Date: Sun, 29 Jan 2023 11:30:21 +0800 Subject: [PATCH 05/18] skip doGCPlacementRules when keyspace_name is set Signed-off-by: ystaticy --- store/gcworker/gc_worker.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/store/gcworker/gc_worker.go b/store/gcworker/gc_worker.go index 8946dc2140e5f..c09d7b8506aa8 100644 --- a/store/gcworker/gc_worker.go +++ b/store/gcworker/gc_worker.go @@ -2058,6 +2058,12 @@ func (w *GCWorker) saveValueToSysTable(key, value string) error { // Placement rules cannot be removed immediately after drop table / truncate table, // because the tables can be flashed back or recovered. func (w *GCWorker) doGCPlacementRules(se session.Session, safePoint uint64, dr util.DelRangeTask, gcPlacementRuleCache map[int64]interface{}) (err error) { + + if w.store.GetCodec().GetKeyspace() != nil { + logutil.BgLogger().Info("[gc worker] skip doGCPlacementRules when keyspace_name is set.") + return nil + } + // Get the job from the job history var historyJob *model.Job failpoint.Inject("mockHistoryJobForGC", func(v failpoint.Value) { From 216b19652b185f83e310770becaeb313647e5b01 Mon Sep 17 00:00:00 2001 From: ystaticy Date: Mon, 30 Jan 2023 17:07:13 +0800 Subject: [PATCH 06/18] remove empty line Signed-off-by: ystaticy --- store/gcworker/gc_worker.go | 1 - 1 file changed, 1 deletion(-) diff --git a/store/gcworker/gc_worker.go b/store/gcworker/gc_worker.go index c09d7b8506aa8..8b0b82efbec03 100644 --- a/store/gcworker/gc_worker.go +++ b/store/gcworker/gc_worker.go @@ -2058,7 +2058,6 @@ func (w *GCWorker) saveValueToSysTable(key, value string) error { // Placement rules cannot be removed immediately after drop table / truncate table, // because the tables can be flashed back or recovered. func (w *GCWorker) doGCPlacementRules(se session.Session, safePoint uint64, dr util.DelRangeTask, gcPlacementRuleCache map[int64]interface{}) (err error) { - if w.store.GetCodec().GetKeyspace() != nil { logutil.BgLogger().Info("[gc worker] skip doGCPlacementRules when keyspace_name is set.") return nil From e36f5914246b0d9daac22635502618fa24691da8 Mon Sep 17 00:00:00 2001 From: ystaticy Date: Tue, 31 Jan 2023 13:07:21 +0800 Subject: [PATCH 07/18] fix conditiosn Signed-off-by: ystaticy --- store/gcworker/gc_worker.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/store/gcworker/gc_worker.go b/store/gcworker/gc_worker.go index 8b0b82efbec03..53034974c94b8 100644 --- a/store/gcworker/gc_worker.go +++ b/store/gcworker/gc_worker.go @@ -315,10 +315,16 @@ func getGCSafePoint(ctx context.Context, pdClient pd.Client) (uint64, error) { func (w *GCWorker) runKeyspaceDeleteRange(ctx context.Context, concurrency int) error { // Get safe point from PD. safePoint, err := getGCSafePoint(ctx, w.pdClient) - if safePoint == 0 { + if err != nil { logutil.Logger(ctx).Info("[gc worker] get gc safe point error", zap.Error(errors.Trace(err))) return nil } + + if safePoint == 0 { + logutil.Logger(ctx).Info("[gc worker] skip keyspace delete range, because gc safe point is 0") + return nil + } + keyspaceID := w.store.GetCodec().GetKeyspaceID() logutil.Logger(ctx).Info("[gc worker] start keyspace delete range", zap.String("uuid", w.uuid), @@ -326,11 +332,6 @@ func (w *GCWorker) runKeyspaceDeleteRange(ctx context.Context, concurrency int) zap.Uint32("keyspaceID", uint32(keyspaceID)), zap.Uint64("GCSafepoint", safePoint)) - if safePoint == 0 { - logutil.Logger(ctx).Info("[gc worker] skip keyspace delete range, because gc safe point is 0") - return nil - } - // Do deleteRanges. err = w.deleteRanges(ctx, safePoint, concurrency) if err != nil { From ddd49a98b26572723ec5ab8972e8c08656672292 Mon Sep 17 00:00:00 2001 From: ystaticy Date: Tue, 31 Jan 2023 14:52:59 +0800 Subject: [PATCH 08/18] close etcdcli Signed-off-by: ystaticy --- domain/domain.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/domain/domain.go b/domain/domain.go index 8481c7798eae7..936f8da41f736 100644 --- a/domain/domain.go +++ b/domain/domain.go @@ -882,6 +882,10 @@ func (do *Domain) Close() { terror.Log(errors.Trace(do.etcdClient.Close())) } + if do.unprefixedEtcdCli != nil { + terror.Log(errors.Trace(do.unprefixedEtcdCli.Close())) + } + do.slowQuery.Close() if do.cancel != nil { do.cancel() From b254cfe6112fde093b0e0bd06c840ba904b750b9 Mon Sep 17 00:00:00 2001 From: ystaticy Date: Tue, 31 Jan 2023 16:40:12 +0800 Subject: [PATCH 09/18] add comments Signed-off-by: ystaticy --- store/gcworker/gc_worker.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/store/gcworker/gc_worker.go b/store/gcworker/gc_worker.go index 53034974c94b8..4aef1f6c86266 100644 --- a/store/gcworker/gc_worker.go +++ b/store/gcworker/gc_worker.go @@ -314,6 +314,9 @@ func getGCSafePoint(ctx context.Context, pdClient pd.Client) (uint64, error) { func (w *GCWorker) runKeyspaceDeleteRange(ctx context.Context, concurrency int) error { // Get safe point from PD. + // The update process of GC safe point is to resolve locks first and then update to pd. + // So, in the following code, the data of the range to be deleted must have been resolved locks, + // the range is safe to be deleted. safePoint, err := getGCSafePoint(ctx, w.pdClient) if err != nil { logutil.Logger(ctx).Info("[gc worker] get gc safe point error", zap.Error(errors.Trace(err))) @@ -324,7 +327,6 @@ func (w *GCWorker) runKeyspaceDeleteRange(ctx context.Context, concurrency int) logutil.Logger(ctx).Info("[gc worker] skip keyspace delete range, because gc safe point is 0") return nil } - keyspaceID := w.store.GetCodec().GetKeyspaceID() logutil.Logger(ctx).Info("[gc worker] start keyspace delete range", zap.String("uuid", w.uuid), From bc2a66e2a55a3a0dddf4aabd5218d229aaad56b7 Mon Sep 17 00:00:00 2001 From: ystaticy Date: Tue, 31 Jan 2023 17:50:17 +0800 Subject: [PATCH 10/18] add comments Signed-off-by: ystaticy --- domain/domain.go | 10 +++---- store/gcworker/gc_worker.go | 54 +++++++++++++++++++++---------------- 2 files changed, 36 insertions(+), 28 deletions(-) diff --git a/domain/domain.go b/domain/domain.go index 936f8da41f736..10f9bc33e9ad4 100644 --- a/domain/domain.go +++ b/domain/domain.go @@ -111,9 +111,9 @@ type Domain struct { SchemaValidator SchemaValidator sysSessionPool *sessionPool exit chan struct{} - etcdClient *clientv3.Client - unprefixedEtcdCli *clientv3.Client - sysVarCache sysVarCache // replaces GlobalVariableCache + etcdClient *clientv3.Client // etcdClient must be used when the logic to each etcd path needs to be executed separately by keyspace, or when keyspace is not set. + unprefixedEtcdCli *clientv3.Client // unprefixedEtcdCli must be used when even different keyspaces need to be used globally. It will never set etcd namespace prefix by keyspace. + sysVarCache sysVarCache // replaces GlobalVariableCache slowQuery *topNSlowQueries expensiveQueryHandle *expensivequery.Handle memoryUsageAlarmHandle *memoryusagealarm.Handle @@ -977,11 +977,11 @@ func (do *Domain) Init( do.etcdClient = cli - noNamespaceCli, err := newEtcdCli(addrs, ebd) + unprefixedEtcdCli, err := newEtcdCli(addrs, ebd) if err != nil { return errors.Trace(err) } - do.unprefixedEtcdCli = noNamespaceCli + do.unprefixedEtcdCli = unprefixedEtcdCli } } diff --git a/store/gcworker/gc_worker.go b/store/gcworker/gc_worker.go index 4aef1f6c86266..4f5fc32c1cc3d 100644 --- a/store/gcworker/gc_worker.go +++ b/store/gcworker/gc_worker.go @@ -375,32 +375,10 @@ func (w *GCWorker) leaderTick(ctx context.Context) error { // Do keyspace delete range if w.store.GetCodec().GetKeyspace() != nil { - // When the worker is just started, or an old GC job has just finished, - // wait a while before starting a new job. - if time.Since(w.lastFinish) < gcWaitTime { - logutil.Logger(ctx).Info("[gc worker] another keyspace gc job has just finished, skipped.", - zap.String("leaderTick on ", w.uuid)) - return nil - } - - now, err := w.getOracleTime() - if err != nil { - return errors.Trace(err) - } - ok, err := w.checkGCInterval(now) - if err != nil || !ok { - return errors.Trace(err) - } - - go func() { - w.done <- w.runKeyspaceDeleteRange(ctx, concurrency) - }() - - err = w.saveTime(gcLastRunTimeKey, now) + err = w.runKeyspaceGCJob(ctx, concurrency) if err != nil { return errors.Trace(err) } - return nil } @@ -441,6 +419,36 @@ func (w *GCWorker) leaderTick(ctx context.Context) error { return nil } +func (w *GCWorker) runKeyspaceGCJob(ctx context.Context, concurrency int) error { + // When the worker is just started, or an old GC job has just finished, + // wait a while before starting a new job. + if time.Since(w.lastFinish) < gcWaitTime { + logutil.Logger(ctx).Info("[gc worker] another keyspace gc job has just finished, skipped.", + zap.String("leaderTick on ", w.uuid)) + return nil + } + + now, err := w.getOracleTime() + if err != nil { + return errors.Trace(err) + } + ok, err := w.checkGCInterval(now) + if err != nil || !ok { + return errors.Trace(err) + } + + go func() { + w.done <- w.runKeyspaceDeleteRange(ctx, concurrency) + }() + + err = w.saveTime(gcLastRunTimeKey, now) + if err != nil { + return errors.Trace(err) + } + + return nil +} + // prepare checks preconditions for starting a GC job. It returns a bool // that indicates whether the GC job should start and the new safePoint. func (w *GCWorker) prepare(ctx context.Context) (bool, uint64, error) { From 6de13aa1ea87cba76c45f4586d168e52b3e9e827 Mon Sep 17 00:00:00 2001 From: ystaticy Date: Tue, 31 Jan 2023 21:16:28 +0800 Subject: [PATCH 11/18] add comments Signed-off-by: ystaticy --- domain/domain.go | 37 +++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/domain/domain.go b/domain/domain.go index 10f9bc33e9ad4..f1dfd8be254aa 100644 --- a/domain/domain.go +++ b/domain/domain.go @@ -98,22 +98,27 @@ func NewMockDomain() *Domain { // Domain represents a storage space. Different domains can use the same database name. // Multiple domains can be used in parallel without synchronization. type Domain struct { - store kv.Storage - infoCache *infoschema.InfoCache - privHandle *privileges.Handle - bindHandle atomic.Pointer[bindinfo.BindHandle] - statsHandle unsafe.Pointer - statsLease time.Duration - ddl ddl.DDL - info *infosync.InfoSyncer - globalCfgSyncer *globalconfigsync.GlobalConfigSyncer - m sync.Mutex - SchemaValidator SchemaValidator - sysSessionPool *sessionPool - exit chan struct{} - etcdClient *clientv3.Client // etcdClient must be used when the logic to each etcd path needs to be executed separately by keyspace, or when keyspace is not set. - unprefixedEtcdCli *clientv3.Client // unprefixedEtcdCli must be used when even different keyspaces need to be used globally. It will never set etcd namespace prefix by keyspace. - sysVarCache sysVarCache // replaces GlobalVariableCache + store kv.Storage + infoCache *infoschema.InfoCache + privHandle *privileges.Handle + bindHandle atomic.Pointer[bindinfo.BindHandle] + statsHandle unsafe.Pointer + statsLease time.Duration + ddl ddl.DDL + info *infosync.InfoSyncer + globalCfgSyncer *globalconfigsync.GlobalConfigSyncer + m sync.Mutex + SchemaValidator SchemaValidator + sysSessionPool *sessionPool + exit chan struct{} + // `etcdClient` must be used when keyspace is not set, or when the logic to each etcd path needs to be separated by keyspace. + etcdClient *clientv3.Client + // `unprefixedEtcdCli` will never set etcd namespace prefix by keyspace. + // It's only used in storeMinStartTS and RemoveMinStartTS now. + // It's must be used when the etcd path isn't need to separate by keyspaces. + // See: https://github.com/pingcap/tidb/pull/39685 + unprefixedEtcdCli *clientv3.Client + sysVarCache sysVarCache // replaces GlobalVariableCache slowQuery *topNSlowQueries expensiveQueryHandle *expensivequery.Handle memoryUsageAlarmHandle *memoryusagealarm.Handle From 9ffe54f8514005c7aebf799eefef4aa05ef9784b Mon Sep 17 00:00:00 2001 From: ystaticy Date: Wed, 1 Feb 2023 21:46:02 +0800 Subject: [PATCH 12/18] add func to print log when gc safepoint is too early Signed-off-by: ystaticy --- store/gcworker/gc_worker.go | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/store/gcworker/gc_worker.go b/store/gcworker/gc_worker.go index 4f5fc32c1cc3d..7632bc8ea83fb 100644 --- a/store/gcworker/gc_worker.go +++ b/store/gcworker/gc_worker.go @@ -312,6 +312,22 @@ func getGCSafePoint(ctx context.Context, pdClient pd.Client) (uint64, error) { return safePoint, nil } +func (w *GCWorker) logIsGCSafePointTooEarly(ctx context.Context, safePoint uint64) error { + now, err := w.getOracleTime() + if err != nil { + return errors.Trace(err) + } + + checkTs := oracle.GoTimeToTS(now.Add(-gcDefaultLifeTime * 2)) + if checkTs > safePoint { + logutil.Logger(ctx).Info("[gc worker] gc safepoint is too early. " + + "Maybe there is a bit BR/Lightning/CDC task, " + + "or a long transaction is running" + + "or need a tidb without setting keyspace-name to calculate and update gc safe point.") + } + return nil +} + func (w *GCWorker) runKeyspaceDeleteRange(ctx context.Context, concurrency int) error { // Get safe point from PD. // The update process of GC safe point is to resolve locks first and then update to pd. @@ -327,6 +343,13 @@ func (w *GCWorker) runKeyspaceDeleteRange(ctx context.Context, concurrency int) logutil.Logger(ctx).Info("[gc worker] skip keyspace delete range, because gc safe point is 0") return nil } + + err = w.logIsGCSafePointTooEarly(ctx, safePoint) + if err != nil { + logutil.Logger(ctx).Info("[gc worker] log is gc safe point is too early error", zap.Error(errors.Trace(err))) + return nil + } + keyspaceID := w.store.GetCodec().GetKeyspaceID() logutil.Logger(ctx).Info("[gc worker] start keyspace delete range", zap.String("uuid", w.uuid), From 5d32161c8ec0749fdf428a24131c2de376b3cdea Mon Sep 17 00:00:00 2001 From: ystaticy Date: Thu, 2 Feb 2023 08:31:43 +0800 Subject: [PATCH 13/18] add uuid Signed-off-by: ystaticy --- store/gcworker/gc_worker.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/store/gcworker/gc_worker.go b/store/gcworker/gc_worker.go index 7632bc8ea83fb..570b5526f9a80 100644 --- a/store/gcworker/gc_worker.go +++ b/store/gcworker/gc_worker.go @@ -2093,7 +2093,7 @@ func (w *GCWorker) saveValueToSysTable(key, value string) error { // because the tables can be flashed back or recovered. func (w *GCWorker) doGCPlacementRules(se session.Session, safePoint uint64, dr util.DelRangeTask, gcPlacementRuleCache map[int64]interface{}) (err error) { if w.store.GetCodec().GetKeyspace() != nil { - logutil.BgLogger().Info("[gc worker] skip doGCPlacementRules when keyspace_name is set.") + logutil.BgLogger().Info("[gc worker] skip doGCPlacementRules when keyspace_name is set.", zap.String("uuid", w.uuid)) return nil } From 790651a20b32b5e2525da47b04b508080173a61f Mon Sep 17 00:00:00 2001 From: ystaticy Date: Thu, 2 Feb 2023 08:54:11 +0800 Subject: [PATCH 14/18] add comments Signed-off-by: ystaticy --- store/gcworker/gc_worker.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/store/gcworker/gc_worker.go b/store/gcworker/gc_worker.go index 570b5526f9a80..de5a18cba2d57 100644 --- a/store/gcworker/gc_worker.go +++ b/store/gcworker/gc_worker.go @@ -396,7 +396,9 @@ func (w *GCWorker) leaderTick(ctx context.Context) error { return errors.Trace(err) } - // Do keyspace delete range + // Gc safe point is not separated by keyspace now, the whole cluster has only one global gc safe point. + // So at least one tidb without set `keyspace-name` is required in the whole cluster to calculate and update gc safe point. + // If TiDB set `keyspace-name` it will only do its own delete range, and will not calculate gc safe point and resolve locks. if w.store.GetCodec().GetKeyspace() != nil { err = w.runKeyspaceGCJob(ctx, concurrency) if err != nil { From 29fd7221abbe9c36cea2eeab2be7824a064bc779 Mon Sep 17 00:00:00 2001 From: ystaticy Date: Thu, 2 Feb 2023 10:06:59 +0800 Subject: [PATCH 15/18] fix grammar Signed-off-by: ystaticy --- domain/domain.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/domain/domain.go b/domain/domain.go index f1dfd8be254aa..5d86652f19be2 100644 --- a/domain/domain.go +++ b/domain/domain.go @@ -114,9 +114,9 @@ type Domain struct { // `etcdClient` must be used when keyspace is not set, or when the logic to each etcd path needs to be separated by keyspace. etcdClient *clientv3.Client // `unprefixedEtcdCli` will never set etcd namespace prefix by keyspace. - // It's only used in storeMinStartTS and RemoveMinStartTS now. - // It's must be used when the etcd path isn't need to separate by keyspaces. - // See: https://github.com/pingcap/tidb/pull/39685 + // It only used in storeMinStartTS and RemoveMinStartTS now. + // It must be used when the etcd path isn't need to separate by keyspace. + // See keyspace RFC: https://github.com/pingcap/tidb/pull/39685 unprefixedEtcdCli *clientv3.Client sysVarCache sysVarCache // replaces GlobalVariableCache slowQuery *topNSlowQueries From 1bf92850baea885a8347ae62526f5492f270ae6c Mon Sep 17 00:00:00 2001 From: ystaticy Date: Thu, 2 Feb 2023 10:09:09 +0800 Subject: [PATCH 16/18] fix grammar Signed-off-by: ystaticy --- domain/domain.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/domain/domain.go b/domain/domain.go index 5d86652f19be2..3f1c833b148c0 100644 --- a/domain/domain.go +++ b/domain/domain.go @@ -113,9 +113,9 @@ type Domain struct { exit chan struct{} // `etcdClient` must be used when keyspace is not set, or when the logic to each etcd path needs to be separated by keyspace. etcdClient *clientv3.Client - // `unprefixedEtcdCli` will never set etcd namespace prefix by keyspace. - // It only used in storeMinStartTS and RemoveMinStartTS now. - // It must be used when the etcd path isn't need to separate by keyspace. + // `unprefixedEtcdCli` will never set the etcd namespace prefix by keyspace. + // It is only used in storeMinStartTS and RemoveMinStartTS now. + // It must be used when the etcd path isn't needed to separate by keyspace. // See keyspace RFC: https://github.com/pingcap/tidb/pull/39685 unprefixedEtcdCli *clientv3.Client sysVarCache sysVarCache // replaces GlobalVariableCache From 0c1cb168c4d214ffa01bf2df721bbabcf9b1f568 Mon Sep 17 00:00:00 2001 From: ystaticy Date: Thu, 2 Feb 2023 10:13:33 +0800 Subject: [PATCH 17/18] add comnments Signed-off-by: ystaticy --- domain/infosync/info.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/domain/infosync/info.go b/domain/infosync/info.go index f6dbecca0817c..6c9e721959cf9 100644 --- a/domain/infosync/info.go +++ b/domain/infosync/info.go @@ -95,7 +95,12 @@ var ErrPrometheusAddrIsNotSet = dbterror.ClassDomain.NewStd(errno.ErrPrometheusA // InfoSyncer stores server info to etcd when the tidb-server starts and delete when tidb-server shuts down. type InfoSyncer struct { - etcdCli *clientv3.Client + // `etcdClient` must be used when keyspace is not set, or when the logic to each etcd path needs to be separated by keyspace. + etcdCli *clientv3.Client + // `unprefixedEtcdCli` will never set the etcd namespace prefix by keyspace. + // It is only used in storeMinStartTS and RemoveMinStartTS now. + // It must be used when the etcd path isn't needed to separate by keyspace. + // See keyspace RFC: https://github.com/pingcap/tidb/pull/39685 unprefixedEtcdCli *clientv3.Client info *ServerInfo serverInfoPath string From af0df85ab195ec159761f06da8ee84d5592ff426 Mon Sep 17 00:00:00 2001 From: ystaticy Date: Thu, 2 Feb 2023 13:21:47 +0800 Subject: [PATCH 18/18] fix comnments Signed-off-by: ystaticy --- store/gcworker/gc_worker.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/store/gcworker/gc_worker.go b/store/gcworker/gc_worker.go index de5a18cba2d57..474096fbca7ec 100644 --- a/store/gcworker/gc_worker.go +++ b/store/gcworker/gc_worker.go @@ -330,9 +330,9 @@ func (w *GCWorker) logIsGCSafePointTooEarly(ctx context.Context, safePoint uint6 func (w *GCWorker) runKeyspaceDeleteRange(ctx context.Context, concurrency int) error { // Get safe point from PD. - // The update process of GC safe point is to resolve locks first and then update to pd. - // So, in the following code, the data of the range to be deleted must have been resolved locks, - // the range is safe to be deleted. + // The GC safe point is updated only after the global GC have done resolveLocks phase globally. + // So, in the following code, resolveLocks must have been done by the global GC on the ranges to be deleted, + // so its safe to delete the ranges. safePoint, err := getGCSafePoint(ctx, w.pdClient) if err != nil { logutil.Logger(ctx).Info("[gc worker] get gc safe point error", zap.Error(errors.Trace(err))) @@ -396,9 +396,11 @@ func (w *GCWorker) leaderTick(ctx context.Context) error { return errors.Trace(err) } - // Gc safe point is not separated by keyspace now, the whole cluster has only one global gc safe point. - // So at least one tidb without set `keyspace-name` is required in the whole cluster to calculate and update gc safe point. - // If TiDB set `keyspace-name` it will only do its own delete range, and will not calculate gc safe point and resolve locks. + // Gc safe point is not separated by keyspace now. The whole cluster has only one global gc safe point. + // So at least one TiDB with `keyspace-name` not set is required in the whole cluster to calculate and update gc safe point. + // If `keyspace-name` is set, the TiDB node will only do its own delete range, and will not calculate gc safe point and resolve locks. + // Note that when `keyspace-name` is set, `checkLeader` will be done within the key space. + // Therefore only one TiDB node in each key space will be responsible to do delete range. if w.store.GetCodec().GetKeyspace() != nil { err = w.runKeyspaceGCJob(ctx, concurrency) if err != nil {