GC : fix issue of delete range could not be executed again if the cle…

…aning rules failed (#53368) (#56991) ref #53369
pingcap · Oct 31, 2024 · c5dc4ba · c5dc4ba
1 parent aad5075
commit c5dc4ba
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 10 deletions.
diff --git a/store/gcworker/gc_worker.go b/store/gcworker/gc_worker.go
@@ -892,16 +892,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("[gc worker] failed to mark delete range task done",
-				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 worker] gc placement rules failed on range",
 				zap.String("uuid", w.uuid),
@@ -918,6 +908,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("[gc worker] failed to mark delete range task done",
+				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("[gc worker] finish delete ranges",
 		zap.String("uuid", w.uuid),

diff --git a/store/gcworker/gc_worker_test.go b/store/gcworker/gc_worker_test.go
@@ -688,6 +688,11 @@ func TestDeleteRangesFailure(t *testing.T) {
 				require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/store/gcworker/mockHistoryJobForGC"))
 			}()
 
+			require.NoError(t, failpoint.Enable("github.com/pingcap/tidb/store/gcworker/mockHistoryJob", "return(\"schema/d1/t1\")"))
+			defer func() {
+				require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/store/gcworker/mockHistoryJob"))
+			}()
+
 			// Put some delete range tasks.
 			se := createSession(s.gcWorker.store)
 			defer se.Close()
@@ -847,6 +852,16 @@ Loop:
 func TestUnsafeDestroyRangeForRaftkv2(t *testing.T) {
 	require.NoError(t, failpoint.Enable("github.com/pingcap/tidb/store/gcworker/isRaftKv2", "return(true)"))
 
+	require.NoError(t, failpoint.Enable("github.com/pingcap/tidb/store/gcworker/mockHistoryJobForGC", "return(1)"))
+	defer func() {
+		require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/store/gcworker/mockHistoryJobForGC"))
+	}()
+
+	require.NoError(t, failpoint.Enable("github.com/pingcap/tidb/store/gcworker/mockHistoryJob", "return(\"schema/d1/t1\")"))
+	defer func() {
+		require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/store/gcworker/mockHistoryJob"))
+	}()
+
 	s := createGCWorkerSuite(t)
 	// Put some delete range tasks.
 	se := createSession(s.gcWorker.store)