Skip to content

Commit 765504b

Browse files
committed
batcheval: reliably remove GCHint when it expires
The GCHint would previously only be adjusted when GC would actually do work. However, conceivably (and reproducably), sometimes all of the GC work would be done under a GCThreshold that did not yet allow the GCHint to be collected. This would leave the GCHint dangling, and endlessly queue the range for GC over and over again. Unconditionally clear the GCHint to avoid this. The problem could reliably be reproduced using an [experiment]. It no longer reproduces as of this commit. [experiment]: https://docs.google.com/document/d/1QajDmDPgICvwYeRmrFVKvqVRW_kwWMD5RAe_q6irCXc/edit?tab=t.0 See also this [Slack thread]. [Slack thread]: https://cockroachlabs.slack.com/archives/G01G8LK77DK/p1757580849424569?thread_ts=1757428752.120299&cid=G01G8LK77DK Epic: none
1 parent 8b1e1a9 commit 765504b

File tree

2 files changed

+7
-3
lines changed

2 files changed

+7
-3
lines changed

pkg/kv/kvserver/batcheval/cmd_gc.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -265,9 +265,7 @@ func GC(
265265
// Check if optional GC hint on the range is expired (e.g. delete operation is
266266
// older than GC threshold) and remove it. Otherwise this range could be
267267
// unnecessarily GC'd with high priority again.
268-
// We should only do that when we are doing actual cleanup as we want to have
269-
// a hint when request is being handled.
270-
if len(args.Keys) != 0 || len(args.RangeKeys) != 0 || args.ClearRange != nil {
268+
{
271269
sl := MakeStateLoader(cArgs.EvalCtx)
272270
hint, err := sl.LoadGCHint(ctx, readWriter)
273271
if err != nil {

pkg/kv/kvserver/mvcc_gc_queue.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -783,6 +783,12 @@ func (mgcq *mvccGCQueue) process(
783783
if scoreAfter.ShouldQueue {
784784
// The scores are very long, so splitting into multiple lines manually for
785785
// readability.
786+
//
787+
// NB: there are likely situations in which this check triggers incorrectly,
788+
// for example when the GC hint triggers GC but a protected timestamp
789+
// prevents the GC threshold from advancing. In that case, not only did we
790+
// run a GC cycle without improving anything, but we also pile up a stats
791+
// recomputation. This is hopefully too rare to matter.
786792
log.Dev.Infof(ctx, "GC still needed following GC, recomputing MVCC stats")
787793
log.Dev.Infof(ctx, "old score %s", r)
788794
log.Dev.Infof(ctx, "new score %s", scoreAfter)

0 commit comments

Comments
 (0)