Skip to content

fix(vfs): stop wedging the level-1 tailer on a straddling first compaction - #1465

Open
yzxcj797 wants to merge 1 commit into
benbjohnson:mainfrom
yzxcj797:fix/vfs-l1-straddle-wedge-1460
Open

fix(vfs): stop wedging the level-1 tailer on a straddling first compaction#1465
yzxcj797 wants to merge 1 commit into
benbjohnson:mainfrom
yzxcj797:fix/vfs-l1-straddle-wedge-1460

Conversation

@yzxcj797

Copy link
Copy Markdown

Fixes #1460.

Investigation (reproduced from the report's timeline)

rebuildIndex seeds the level-1 tailer position from an L0-derived value when the index is built while L1 is still empty:

maxTXID1 := maxLevelTXID(infos, 1)
if maxTXID1 == 0 {
    maxTXID1 = pos.TXID   // e.g. 6 — NOT an L1 file boundary
}

The writer's first L0→L1 compaction then emits a file whose range straddles that value (1..0xb around position 6). The level-1 seek filters by MinTXID, so the straddling file is invisible forever; when the next L1 file (0xc..0x1d) appears, the strict contiguity check fails on every poll and the follower freezes at TXID 6 while storage remains fully contiguous:

poll L1: non-contiguous ltx file: level=1, current=0000000000000006, next=000000000000000c-000000000000001d

Fix

maxTXID1 stays zero when there are no L1 files, so the first L1 file — which starts at TXID 1 regardless of how far L0 has advanced — is consumed contiguously, and the position tracks real L1 boundaries from then on.

Scope

  • This PR changes only the seeding in rebuildIndex (plus tests). It does not change the MinTXID-based seek semantics of any storage backend, nor the pollLevel contiguity rules — with a correctly-zero seed neither can encounter a straddle.
  • The vfs-tagged test suite did not compile on main (the in-file test mocks predate the SetLogger addition to ReplicaClient); the three mocks get a no-op SetLogger so the tagged suite builds again. Pre-existing failures unrelated to this change (e.g. TestReplica_SyncOnceLimitsLTXFiles, TestDB_SyncChunksWALAtCommitBoundary) reproduce identically with and without this fix.

Tests

Updated TestVFSFile_OpenSeedsLevel1PositionFromPos (which pinned the old seeding) to TestVFSFile_OpenKeepsLevel1PositionAtZeroWithoutL1Files, and added TestVFSFile_PollConsumesStraddlingL1Compaction, which replays the report's exact timeline: open with L0 at 6 and L1 empty, then present L1 1..0xb and 0xc..0x1d, then poll.

go test -tags vfs -run 'TestVFSFile_PollConsumesStraddlingL1Compaction|TestVFSFile_OpenKeepsLevel1PositionAtZero' -count=1 .
# ok

Differential: reverting only the seeding (tests kept) fails with exactly the report's wedge error — poll L1: non-contiguous ltx file: level=1, current=0000000000000006, next=000000000000000c-000000000000001d.

…ction

rebuildIndex seeded maxTXID1 from the L0-derived pos.TXID when the
index was built while L1 was still empty. That value is not an L1
boundary: the writer's first L0-to-L1 compaction emits a file whose
TXID range straddles it (e.g. 1..0xb around a position of 6). The
level-1 seek skips files by MinTXID, so the straddling file is never
returned, and once the next L1 file appears (0xc..0x1d) the strict
contiguity check fails forever:

  poll L1: non-contiguous ltx file: level=1, current=6, next=c-1d

The follower freezes at TXID 6 while still serving stale reads, even
though storage is fully contiguous.

Leave maxTXID1 at zero when there are no L1 files, so the first L1
file is consumed contiguously from TXID 1 and the position tracks real
L1 boundaries from then on.

The vfs-tagged test build was broken on main (the test-file mocks
predate the SetLogger addition to ReplicaClient); the mocks are updated
so the tagged suite compiles again.

Fixes benbjohnson#1460
@darkgnotic

Copy link
Copy Markdown
Contributor

Thank you @yzxcj797 . 🙏

FWIW, I asked Claude about this solution and it seemed to think that it would break compaction in write mode (e.g. introduced in #953). But TBH, I would love for this to be the correct solution, as it's much simpler. 🤞

@yzxcj797

Copy link
Copy Markdown
Author

@darkgnotic thanks for flagging that — I checked the write-mode path against #953 and I don't see an interaction:

  • maxTXID1 is consumed in exactly one place: poll()pollLevel(ctx, 1, f.maxTXID1, ...) (vfs.go:2511/2538), the read-side L1 tailer. The only other reference is the MaxTXID1() accessor, which has no callers outside tests.
  • The write-mode compactor (compactor.go) never reads f.maxTXID1; its level/retention decisions come from the replica client listing and its own state, so a zero seed can't steer compaction.
  • In write mode the poll goroutine still runs ("in case another writer creates the database"). With the zero seed it consumes the first L1 compaction output (one-time, bounded by the first compaction's files) instead of skipping past it; from then on it tracks real L1 boundaries exactly as before. Page-index fetches that hit a file being compacted away fail the poll and retry next tick — the same failure/retry mode the pre-existing fresh-consumption path already has.

So the effect is strictly within the read-side tailer that the issue is about.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

vfs gets wedged on non-contiguous ltx file when an L1 compaction straddles maxTXID1

2 participants