fix(mount): count manifest sizes in merge condition to prevent manifest accumulation

shouldMergeChunks only counted non-manifest chunk sizes toward the bloat
threshold, so overlapping manifests accumulated undetected across flush
cycles.

During sustained random writes (e.g. fio), each metadata flush compacts
non-manifest chunks and may group them into a new manifest via
MaybeManifestize, while carrying forward all existing manifests. Since
the merge condition only checked non-manifest totals against 2x file
size, the growing pile of redundant manifests never triggered a merge.

In a real workload: 4 GB file, 25 manifests each covering ~4 GB
(107 GB manifest data on volume servers), but shouldMergeChunks saw only
4.2 GB of non-manifest chunks vs the 8.6 GB threshold -- no merge.

Fix: include manifest coverage sizes in totalChunkSize. This correctly
detects the 111 GB total vs 8.6 GB threshold and triggers the merge,
which re-reads the file as clean chunks and lets the filer's MinusChunks
(which resolves manifests) garbage-collect all redundant sub-chunks.
This commit is contained in:
Chris Lu
2026-04-16 03:18:27 -07:00
parent f9df187928
commit 4f1d3a1f84
2 changed files with 40 additions and 7 deletions
+7
View File
@@ -268,6 +268,13 @@ func shouldMergeChunks(compactedChunks []*filer_pb.FileChunk, manifestChunks []*
for _, chunk := range compactedChunks {
totalChunkSize += chunk.Size
}
// Count manifest coverage toward stored total. Each manifest holds
// sub-chunks on volume servers that cover approximately Size bytes.
// Without this, overlapping manifests accumulate undetected because
// the merge condition only saw the (small) non-manifest chunk total.
for _, chunk := range manifestChunks {
totalChunkSize += chunk.Size
}
allChunks := make([]*filer_pb.FileChunk, 0, len(compactedChunks)+len(manifestChunks))
allChunks = append(allChunks, compactedChunks...)
allChunks = append(allChunks, manifestChunks...)
+33 -7
View File
@@ -68,7 +68,8 @@ func TestShouldMergeChunks_JustOverDouble(t *testing.T) {
}
func TestShouldMergeChunks_ManifestExtendFileSize(t *testing.T) {
// Compacted chunks are small relative to file extended by manifest.
// One manifest extends the file. Total stored = 100 (regular) + 900
// (manifest) = 1000 vs 2*1000 = 2000 → no merge.
compacted := []*filer_pb.FileChunk{
{Offset: 0, Size: 100, FileId: "a", ModifiedTsNs: 1},
}
@@ -77,12 +78,13 @@ func TestShouldMergeChunks_ManifestExtendFileSize(t *testing.T) {
}
_, _, merge := shouldMergeChunks(compacted, manifest)
if merge {
t.Fatal("manifest-extended file should not merge when compacted chunks are small")
t.Fatal("single manifest extending file should not merge")
}
}
func TestShouldMergeChunks_ManifestChunksSizeIgnored(t *testing.T) {
// Only compacted chunk sizes count toward totalChunkSize.
func TestShouldMergeChunks_ManifestSizesCounted(t *testing.T) {
// Manifest sizes must count toward totalChunkSize so that overlapping
// manifests trigger merge.
compacted := []*filer_pb.FileChunk{
{Offset: 0, Size: 100, FileId: "a", ModifiedTsNs: 1},
}
@@ -90,11 +92,35 @@ func TestShouldMergeChunks_ManifestChunksSizeIgnored(t *testing.T) {
{Offset: 0, Size: 100, FileId: "m", ModifiedTsNs: 2, IsChunkManifest: true},
}
total, _, merge := shouldMergeChunks(compacted, manifest)
if total != 100 {
t.Fatalf("totalChunkSize should only count compacted, got %d", total)
if total != 200 {
t.Fatalf("totalChunkSize should count compacted + manifests, got %d", total)
}
if merge {
t.Fatal("should not merge")
t.Fatal("2x total on 100-byte file should not merge (need >2x)")
}
}
func TestShouldMergeChunks_AccumulatedManifests(t *testing.T) {
// Simulates the real bug: multiple manifests each covering the full file
// accumulate across flush cycles. Without counting manifest sizes, the
// merge condition never fires and storage bloats indefinitely.
compacted := []*filer_pb.FileChunk{
{Offset: 0, Size: 1000, FileId: "a", ModifiedTsNs: 100},
}
// 5 manifests each covering the full file — successive flush cycles
var manifests []*filer_pb.FileChunk
for i := 0; i < 5; i++ {
manifests = append(manifests, &filer_pb.FileChunk{
Offset: 0, Size: 1000, FileId: string(rune('m' + i)),
IsChunkManifest: true,
})
}
total, fileSize, merge := shouldMergeChunks(compacted, manifests)
// total = 1000 (regular) + 5*1000 (manifests) = 6000
// fileSize = 1000
// 6000 > 2*1000 → merge
if !merge {
t.Fatalf("accumulated overlapping manifests should trigger merge (total=%d fileSize=%d)", total, fileSize)
}
}