Merge pull request #113040 from cockroachdb/blathers/backport-release…

…-23.2-112948
cockroachdb · Oct 30, 2023 · e041077 · e041077
2 parents 1e5be54 + 829cb6d
commit e041077
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 18 deletions.
diff --git a/pkg/kv/kvserver/batcheval/cmd_delete_range.go b/pkg/kv/kvserver/batcheval/cmd_delete_range.go
@@ -14,19 +14,30 @@ import (
 	"context"
 	"time"
 
+	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/keys"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvpb"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/batcheval/result"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvserverpb"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/lockspanset"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/spanset"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
+	"github.com/cockroachdb/cockroach/pkg/settings"
 	"github.com/cockroachdb/cockroach/pkg/storage"
 	"github.com/cockroachdb/cockroach/pkg/storage/enginepb"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/cockroachdb/errors"
 )
 
+// enableStickyGCHint controls whether the sticky GCHint is enabled.
+var enableStickyGCHint = settings.RegisterBoolSetting(
+	settings.SystemOnly,
+	"kv.gc.sticky_hint.enabled",
+	"enable writing sticky GC hints which expedite garbage collection after schema changes"+
+		" (ignored and assumed 'true' in 23.2)",
+	false,
+)
+
 func init() {
 	RegisterReadWriteCommand(kvpb.DeleteRange, declareKeysDeleteRange, DeleteRange)
 }
@@ -135,8 +146,14 @@ func DeleteRange(
 				return err
 			}
 
-			// Add the timestamp to GCHint to guarantee that GC eventually clears it.
-			updated := hint.ScheduleGCFor(h.Timestamp)
+			updated := false
+			// TODO(pavelkalinnikov): deprecate the cluster setting and call
+			// ScheduleGCFor unconditionally when min supported version is 23.2.
+			if cArgs.EvalCtx.ClusterSettings().Version.IsActive(ctx, clusterversion.V23_2) ||
+				enableStickyGCHint.Get(&cArgs.EvalCtx.ClusterSettings().SV) {
+				// Add the timestamp to GCHint to guarantee that GC eventually clears it.
+				updated = hint.ScheduleGCFor(h.Timestamp)
+			}
 			// If the range tombstone covers the whole Range key span, update the
 			// corresponding timestamp in GCHint to enable ClearRange optimization.
 			if args.Key.Equal(desc.StartKey.AsRawKey()) && args.EndKey.Equal(desc.EndKey.AsRawKey()) {
@@ -147,7 +164,7 @@ func DeleteRange(
 				return nil
 			}
 
-			if updated, err := sl.SetGCHint(ctx, readWriter, cArgs.Stats, hint); err != nil || !updated {
+			if err := sl.SetGCHint(ctx, readWriter, cArgs.Stats, hint); err != nil {
 				return err
 			}
 			res.Replicated.State = &kvserverpb.ReplicaState{

diff --git a/pkg/kv/kvserver/batcheval/cmd_end_transaction.go b/pkg/kv/kvserver/batcheval/cmd_end_transaction.go
@@ -1354,14 +1354,11 @@ func mergeTrigger(
 			return result.Result{}, err
 		}
 		if lhsHint.Merge(rhsHint, rec.GetMVCCStats().HasNoUserData(), merge.RightMVCCStats.HasNoUserData()) {
-			updated, err := lhsLoader.SetGCHint(ctx, batch, ms, lhsHint)
-			if err != nil {
+			if err := lhsLoader.SetGCHint(ctx, batch, ms, lhsHint); err != nil {
 				return result.Result{}, err
 			}
-			if updated {
-				pd.Replicated.State = &kvserverpb.ReplicaState{
-					GCHint: lhsHint,
-				}
+			pd.Replicated.State = &kvserverpb.ReplicaState{
+				GCHint: lhsHint,
 			}
 		}
 	}

diff --git a/pkg/kv/kvserver/batcheval/cmd_gc.go b/pkg/kv/kvserver/batcheval/cmd_gc.go
@@ -285,7 +285,7 @@ func GC(
 				res.Replicated.State = &kvserverpb.ReplicaState{}
 			}
 			res.Replicated.State.GCHint = hint
-			if _, err := sl.SetGCHint(ctx, readWriter, cArgs.Stats, hint); err != nil {
+			if err := sl.SetGCHint(ctx, readWriter, cArgs.Stats, hint); err != nil {
 				return result.Result{}, err
 			}
 		}

diff --git a/pkg/kv/kvserver/stateloader/stateloader.go b/pkg/kv/kvserver/stateloader/stateloader.go
@@ -124,7 +124,7 @@ func (rsl StateLoader) Save(
 	if err := rsl.SetGCThreshold(ctx, readWriter, ms, state.GCThreshold); err != nil {
 		return enginepb.MVCCStats{}, err
 	}
-	if _, err := rsl.SetGCHint(ctx, readWriter, ms, state.GCHint); err != nil {
+	if err := rsl.SetGCHint(ctx, readWriter, ms, state.GCHint); err != nil {
 		return enginepb.MVCCStats{}, err
 	}
 	// TODO(sep-raft-log): SetRaftTruncatedState will be in a separate batch when
@@ -327,15 +327,12 @@ func (rsl StateLoader) LoadGCHint(
 // SetGCHint writes the GC hint.
 func (rsl StateLoader) SetGCHint(
 	ctx context.Context, readWriter storage.ReadWriter, ms *enginepb.MVCCStats, hint *roachpb.GCHint,
-) (updated bool, _ error) {
+) error {
 	if hint == nil {
-		return false, errors.New("cannot persist nil GCHint")
-	}
-	if err := storage.MVCCPutProto(ctx, readWriter, rsl.RangeGCHintKey(),
-		hlc.Timestamp{}, hint, storage.MVCCWriteOptions{Stats: ms}); err != nil {
-		return false, err
+		return errors.New("cannot persist nil GCHint")
 	}
-	return true, nil
+	return storage.MVCCPutProto(ctx, readWriter, rsl.RangeGCHintKey(),
+		hlc.Timestamp{}, hint, storage.MVCCWriteOptions{Stats: ms})
 }
 
 // LoadVersion loads the replica version.