Skip to content

Commit 80da87c

Browse files
crl-release-26.2: manifest: fix BackingValueSize lost during version edit round-trip
When encoding blob references for virtual tables, if BackingValueSize equaled ValueSize for all references, the encoder used customTagBlobReferences (which omits BackingValueSize). On decode, BackingValueSize became 0 — a sentinel meaning "no backing value size info available." After manifest replay, this caused blob files to be incorrectly marked ineligible for rewrite compaction (via virtualReferencesWithoutBackingValueSize), preventing blob garbage collection. Fix the encoding condition to write BackingValueSize whenever it is known (> 0) for virtual tables, regardless of whether it equals ValueSize. Also use max(BackingValueSize, ValueSize) when computing ReferencedBlobValueSizeTotal during accumulation, for robustness against old-format data. Co-Authored-By: roachdev-claude <roachdev-claude-bot@cockroachlabs.com>
1 parent 2fdf467 commit 80da87c

File tree

2 files changed

+73
-2
lines changed

2 files changed

+73
-2
lines changed

internal/manifest/version_edit.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -951,7 +951,7 @@ func (v *VersionEdit) Encode(w io.Writer) error {
951951
writeBackingValueSize := false
952952
if x.Meta.Virtual {
953953
for _, ref := range x.Meta.BlobReferences {
954-
if ref.BackingValueSize > 0 && ref.BackingValueSize != ref.ValueSize {
954+
if ref.BackingValueSize > 0 {
955955
writeBackingValueSize = true
956956
break
957957
}
@@ -1247,7 +1247,7 @@ func (b *BulkVersionEdit) Accumulate(ve *VersionEdit) error {
12471247
// blob references.
12481248
if backing.ReferencedBlobValueSizeTotal == 0 {
12491249
for _, br := range nf.Meta.BlobReferences {
1250-
backing.ReferencedBlobValueSizeTotal += br.BackingValueSize
1250+
backing.ReferencedBlobValueSizeTotal += max(br.BackingValueSize, br.ValueSize)
12511251
}
12521252
}
12531253
nf.Meta.AttachVirtualBacking(backing)

internal/manifest/version_edit_test.go

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,77 @@ func TestVERoundTripAndAccumulate(t *testing.T) {
124124
}
125125
}
126126

127+
// TestVERoundTripBackingValueSizeEqualValueSize verifies that BackingValueSize
128+
// is preserved through encode/decode when it equals ValueSize for a virtual
129+
// table. This is a regression test for a bug where the encoder omitted
130+
// BackingValueSize when it equaled ValueSize, causing it to decode as 0 (the
131+
// sentinel for "unknown").
132+
func TestVERoundTripBackingValueSizeEqualValueSize(t *testing.T) {
133+
cmp := base.DefaultComparer.Compare
134+
m1 := (&TableMetadata{
135+
TableNum: 810,
136+
Size: 8090,
137+
CreationTime: 809060,
138+
SeqNums: base.SeqNumRange{Low: 9, High: 11},
139+
LargestSeqNumAbsolute: 11,
140+
BlobReferences: []BlobReference{
141+
{FileID: 900, ValueSize: 1024, BackingValueSize: 1024},
142+
{FileID: 910, ValueSize: 8090, BackingValueSize: 8090},
143+
},
144+
}).ExtendPointKeyBounds(
145+
cmp,
146+
base.MakeInternalKey([]byte("a"), 0, base.InternalKeyKindSet),
147+
base.MakeInternalKey([]byte("m"), 0, base.InternalKeyKindSet),
148+
)
149+
m1.InitPhysicalBacking()
150+
151+
// Virtual table where BackingValueSize == ValueSize for all references.
152+
m2 := (&TableMetadata{
153+
TableNum: 812,
154+
Size: 8090,
155+
CreationTime: 809060,
156+
SeqNums: base.SeqNumRange{Low: 9, High: 11},
157+
LargestSeqNumAbsolute: 11,
158+
Virtual: true,
159+
BlobReferences: []BlobReference{
160+
{FileID: 900, ValueSize: 1024, BackingValueSize: 1024},
161+
{FileID: 910, ValueSize: 8090, BackingValueSize: 8090},
162+
},
163+
}).ExtendPointKeyBounds(
164+
cmp,
165+
base.MakeInternalKey([]byte("a"), 0, base.InternalKeyKindSet),
166+
base.MakeInternalKey([]byte("c"), 0, base.InternalKeyKindSet),
167+
)
168+
m2.AttachVirtualBacking(m1.TableBacking)
169+
170+
ve1 := VersionEdit{
171+
CreatedBackingTables: []*TableBacking{m1.TableBacking},
172+
NewTables: []NewTableEntry{
173+
{
174+
Level: 4,
175+
Meta: m2,
176+
BackingFileNum: m2.TableBacking.DiskFileNum,
177+
},
178+
},
179+
}
180+
buf := new(bytes.Buffer)
181+
require.NoError(t, ve1.Encode(buf))
182+
var ve2 VersionEdit
183+
require.NoError(t, ve2.Decode(buf))
184+
var bve BulkVersionEdit
185+
require.NoError(t, bve.Accumulate(&ve2))
186+
187+
// Verify BackingValueSize is preserved after round-trip.
188+
require.Equal(t, len(ve1.NewTables), len(ve2.NewTables))
189+
for i, nt := range ve2.NewTables {
190+
for j, ref := range nt.Meta.BlobReferences {
191+
orig := ve1.NewTables[i].Meta.BlobReferences[j]
192+
require.Equal(t, orig.BackingValueSize, ref.BackingValueSize,
193+
"BlobReferences[%d].BackingValueSize: got %d, want %d", j, ref.BackingValueSize, orig.BackingValueSize)
194+
}
195+
}
196+
}
197+
127198
func TestVersionEditRoundTrip(t *testing.T) {
128199
cmp := base.DefaultComparer.Compare
129200
m1 := (&TableMetadata{

0 commit comments

Comments
 (0)