From a865c866ebd6027bdecac113b4caf76b44f7151d Mon Sep 17 00:00:00 2001 From: ystaticy Date: Thu, 6 Jun 2024 15:26:56 +0800 Subject: [PATCH] GC : fix issue of delete range could not be executed again if the cleaning rules failed (#53368) ref pingcap/tidb#53369 --- pkg/store/gcworker/gc_worker.go | 20 ++++++++++---------- pkg/store/gcworker/gc_worker_test.go | 15 +++++++++++++++ 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/pkg/store/gcworker/gc_worker.go b/pkg/store/gcworker/gc_worker.go index 96f1c67f38c4f..e05099726701b 100644 --- a/pkg/store/gcworker/gc_worker.go +++ b/pkg/store/gcworker/gc_worker.go @@ -845,16 +845,6 @@ func (w *GCWorker) deleteRanges(ctx context.Context, safePoint uint64, concurren continue } - err = util.CompleteDeleteRange(se, r, !v2) - if err != nil { - logutil.Logger(ctx).Error("failed to mark delete range task done", zap.String("category", "gc worker"), - zap.String("uuid", w.uuid), - zap.Stringer("startKey", startKey), - zap.Stringer("endKey", endKey), - zap.Error(err)) - metrics.GCUnsafeDestroyRangeFailuresCounterVec.WithLabelValues("save").Inc() - } - if err := w.doGCPlacementRules(se, safePoint, r, gcPlacementRuleCache); err != nil { logutil.Logger(ctx).Error("gc placement rules failed on range", zap.String("category", "gc worker"), zap.String("uuid", w.uuid), @@ -871,6 +861,16 @@ func (w *GCWorker) deleteRanges(ctx context.Context, safePoint uint64, concurren zap.Error(err)) continue } + + err = util.CompleteDeleteRange(se, r, !v2) + if err != nil { + logutil.Logger(ctx).Error("failed to mark delete range task done", zap.String("category", "gc worker"), + zap.String("uuid", w.uuid), + zap.Stringer("startKey", startKey), + zap.Stringer("endKey", endKey), + zap.Error(err)) + metrics.GCUnsafeDestroyRangeFailuresCounterVec.WithLabelValues("save").Inc() + } } logutil.Logger(ctx).Info("finish delete ranges", zap.String("category", "gc worker"), zap.String("uuid", w.uuid), diff --git a/pkg/store/gcworker/gc_worker_test.go b/pkg/store/gcworker/gc_worker_test.go index 7961ed695700a..b7689191094fe 100644 --- a/pkg/store/gcworker/gc_worker_test.go +++ b/pkg/store/gcworker/gc_worker_test.go @@ -654,6 +654,11 @@ func TestDeleteRangesFailure(t *testing.T) { require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/pkg/store/gcworker/mockHistoryJobForGC")) }() + require.NoError(t, failpoint.Enable("github.com/pingcap/tidb/pkg/store/gcworker/mockHistoryJob", "return(\"schema/d1/t1\")")) + defer func() { + require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/pkg/store/gcworker/mockHistoryJob")) + }() + // Put some delete range tasks. se := createSession(s.gcWorker.store) defer se.Close() @@ -813,6 +818,16 @@ Loop: func TestUnsafeDestroyRangeForRaftkv2(t *testing.T) { require.NoError(t, failpoint.Enable("github.com/pingcap/tidb/pkg/ddl/util/IsRaftKv2", "return(true)")) + require.NoError(t, failpoint.Enable("github.com/pingcap/tidb/pkg/store/gcworker/mockHistoryJobForGC", "return(1)")) + defer func() { + require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/pkg/store/gcworker/mockHistoryJobForGC")) + }() + + require.NoError(t, failpoint.Enable("github.com/pingcap/tidb/pkg/store/gcworker/mockHistoryJob", "return(\"schema/d1/t1\")")) + defer func() { + require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/pkg/store/gcworker/mockHistoryJob")) + }() + s := createGCWorkerSuite(t) // Put some delete range tasks. se := createSession(s.gcWorker.store)