Repository navigation
fix(wal): line-protocol WAL entries carry their database envelope (#889) - #1055
Conversation
d2dc072 to
76d8d90
Compare
… and registrar gates; add the regression test and note Review fold-in on top of the contributor's change (Basekick-Labs#1055): - Drop the cmd/arc/main.go hunk (Basekick-Labs#886). Starting a receiver on readers and compactors is the switch that turns Basekick-Labs#888 live: a replica applies replicated rows to its own ArrowBuffer, flushes them to its own storage under a unique name, and its queries read that file alongside the copy pulled from the primary (read_parquet globs storage; the query handler never reads the buffer). On a compactor the unannounced copy is merged into compacted output that is announced through the manifest bridge. Basekick-Labs#886 stays closed until the Basekick-Labs#888 hand-off lands. - Drop the internal/cluster/file_registrar.go role gate. It removes the announcement, not the file, so it does not address Basekick-Labs#888, and it would silently stop announcing files the /api/v1/import/* endpoints write on a reader or compactor (those endpoints have no write forwarding and no role check, so today their flushes are announced and pulled). That seam needs the design pass with Basekick-Labs#886/Basekick-Labs#888. - Keep the row-format WAL envelope (Basekick-Labs#889). Replay-safe (the reader parses the envelope and tries the row array first; recovery routes by the _database stamped on each record), receiver-safe, checkpoint-safe (same tracked tokens), oversize-safe (array split), and readable by every binary from 26.05.1. - Test: TestLineProtocolWALEntriesCarryTheirDatabase captures the replication-hook payload for a WriteColumnarDirect write with no raw bytes, checks that ParseEnvelope(payload, "default") resolves to the real database and the inner payload is a row array, and that the on-disk entry still replays as a tracked row entry. Fails on main. - Release note in RELEASE_NOTES_2026.09.3.md under Bug fixes, saying why Basekick-Labs#886 and Basekick-Labs#888 are not in this release and what a reader that already receives will observe.
76d8d90 to
123a6d2
Compare
|
Thanks for taking these on, @lecodev-26. I reviewed the three changes against the configurations they reach and landed the one that is sound on its own. Here is what changed and why. Kept: the #889 envelope on the row-format WAL path. Correct and exactly the right mechanism. Replay-safe ( Dropped: the
Dropped: the registrar role gate. It removes the announcement, not the file, so it does not change what the replica reads. And it has a victim: the So #886 and #888 stay open and #889 closes. One consequence to be upfront about: on a cluster whose readers already receive (a local WAL enabled on the reader), line-protocol rows now land in the right database, and the reader still flushes and announces its own copy, so until #888 lands they count twice on every node, as native msgpack rows already do. The 26.09.3 release note says so. I retitled the PR to match what ships. Appreciate the clean diff; the envelope fix was exactly right. @drakeo338 proposed this same source-side envelope in #889 on 2026-09-23 and asked how to sequence it; the note credits both of you, and sorry for the silence on that question. |
xe-nvdk
left a comment
There was a problem hiding this comment.
Reviewed against the configuration matrix (standalone, Pattern 1 writer/reader/compactor, Pattern 2); the #889 envelope is replay-, receiver-, checkpoint- and oversize-safe, with a regression test that fails on main. The #886 and #888 hunks were dropped in review (see the comment above). CI green on 123a6d2.
Fixes #889.
Line-protocol writes reach the buffer as columns with no raw client bytes, so their WAL entry took the row-format fallback and went out bare: the replication stream carried no database, and a receiver parsing it with
ParseEnvelope(payload, "default")filed every row underdefault. The row-format path now writes the same[0x01][dbLen][db][msgpack]envelope the msgpack path has always used.Replay-safe (the reader parses the envelope and tries the row array first; recovery routes by the
_databasestamped on each record), receiver-safe, checkpoint-safe (same tracked identities), oversize-safe (arrays split by element), readable by every binary from 26.05.1.Scope note: the original branch also started a replication receiver on readers/compactors without a local WAL (#886) and gated the file registrar by role (#888). Both were dropped in review; see the review comment. #886 and #888 stay open for the local-storage replication design pass.
Test plan
TestLineProtocolWALEntriesCarryTheirDatabasefails onmain, passes here (20× alone under-race)go test -tags=duckdb_arrow -race ./internal/ingest/ ./internal/wal/ ./internal/cluster/gofmt -l,go vet -tags=duckdb_arrow,go build -tags=duckdb_arrow ./cmd/... ./internal/...RELEASE_NOTES_2026.09.3.md