#2334·badger

RunValueLogGC gives up when the active vlog file tops the discard table, skipping reclaimable older files

Author: matthewmcneelyCreated Aug 18, 2026Updated Aug 18, 2026
Labelskind/bugpriority/P2area/gc

Summary

valueLog.pickLog considers only the single highest-discard file. When that file happens to be the active (write) value log, the call gives up and returns ErrNoRewrite, and older files holding plenty of reclaimable space are never examined. The stall lasts until the active file rolls over, which on defaults means up to ValueLogFileSize (1 GB) of writes.

Mechanism

value.go:1033-1067:

go
fid, discard := vlog.discardStats.MaxDiscard()   // exactly one candidate
...
if thr := discardRatio * float64(fi.Size()); float64(discard) < thr {
    return nil                                   // misses ratio -> whole call fails
}
if fid < vlog.maxFid {
    return lf
}

// Don't randomly pick any value log file.
return nil                                       // top candidate is the active file -> whole call fails

Two things combine:

  1. MaxDiscard() (discard.go:145-157) returns exactly one fid, the one with the largest accumulated discard.
  2. Excluding the active file from rewriting is correct, but the exclusion falls through to return nil rather than moving on to the next-best candidate.

There is no fallthrough, so a single ineligible top candidate fails the entire call. The discardRatio check just above has the same shape: if the top candidate misses the ratio, pickLog returns nil instead of trying the runner-up.

The active file does accumulate discard stats. Compaction credits discardStats[vp.Fid] for every dropped entry (levels.go:682, flushed at :884), and entries written into the active file can be overwritten or deleted and compacted away while that file is still current. On an overwrite-heavy workload the active file is a plausible top candidate for much of its 1 GB lifetime.

Impact

  • GC silently does nothing on a database that has reclaimable space, with only a Debugf line to say why.
  • The documented usage pattern compounds it. docs/quickstart.md tells callers to re-run while the error is nil and stop otherwise, so the first ErrNoRewrite ends the loop and the eligible older files are never reached on that pass either.
  • Worth attention for Dgraph, which leans on value log GC to keep p directories from growing.

This isn't data loss or a crash, and the space is reclaimed eventually once the active file rolls over, so P2 seems about right unless someone has a workload where it bites harder.

Suggested fix

Walk candidates in descending discard order and skip the ineligible ones (active file, or below the ratio) rather than aborting on the first. Either build a sorted candidate list from discardStats.Iterate, or loop MaxDiscard with already-rejected fids masked out.

How this surfaced

Found while verifying the doc claims in #2328, which documents the active-file exclusion as one reason GC reclaims nothing. That documentation is accurate; this is the sharper edge sitting behind it.