RunValueLogGC gives up when the active vlog file tops the discard table, skipping reclaimable older files
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
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 failsTwo things combine:
MaxDiscard()(discard.go:145-157) returns exactly one fid, the one with the largest accumulated discard.- Excluding the active file from rewriting is correct, but the exclusion falls through to
return nilrather 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
Debugfline to say why. - The documented usage pattern compounds it.
docs/quickstart.mdtells callers to re-run while the error is nil and stop otherwise, so the firstErrNoRewriteends 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
pdirectories 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.
Source: dgraph-io/badger