Replace DuckLake with authoritative Parquet storage - #207
Merged
Conversation
Move telemetry ingestion to a durable WAL-backed repository that commits indexed hot segments and open Parquet files. Keep DuckDB as the analytical query and rollup engine, and SQLite as control-plane storage. BREAKING CHANGE: existing DuckLake telemetry data is not migrated. Deployments must start with a clean storage.data_dir.
vishr
commented
Aug 26, 2026
vishr
commented
Aug 26, 2026
vishr
commented
Aug 26, 2026
vishr
commented
Aug 26, 2026
vishr
commented
Aug 26, 2026
vishr
commented
Aug 26, 2026
vishr
commented
Aug 26, 2026
vishr
commented
Aug 26, 2026
Fail fast on legacy DuckLake data instead of hiding it. Drain and publish compaction without blocking reads or commits, and surface failed writes immediately.
vishr
commented
Aug 26, 2026
vishr
commented
Aug 26, 2026
vishr
commented
Aug 26, 2026
vishr
commented
Aug 26, 2026
vishr
commented
Aug 26, 2026
vishr
commented
Aug 26, 2026
Keep failed ingest batches backpressured until durable commit. Use day-partitioned leveled compaction so retention remains enforceable without rewriting the retained corpus. Hold Parquet snapshot locks through row iteration and bound hot-segment descriptors.
vishr
commented
Aug 26, 2026
vishr
commented
Aug 26, 2026
vishr
commented
Aug 26, 2026
vishr
commented
Aug 26, 2026
vishr
commented
Aug 26, 2026
vishr
commented
Aug 26, 2026
vishr
commented
Aug 26, 2026
vishr
commented
Aug 26, 2026
Persist staged directory entries before publishing the recovery marker. Validate every required signal output and restore retired inputs instead of publishing an incomplete compaction.
Prevent compacted WAL replay, bound writer shutdown and poison-batch retries, tier cold log reads through Parquet, and serialize DuckDB maintenance with rollups and context-aware query locks.
vishr
commented
Aug 26, 2026
vishr
commented
Aug 26, 2026
vishr
commented
Aug 26, 2026
vishr
commented
Aug 28, 2026
vishr
commented
Aug 28, 2026
vishr
commented
Aug 28, 2026
vishr
commented
Aug 28, 2026
vishr
commented
Aug 28, 2026
Apply maintenance budgets only between completed publications so slow compactions make durable progress. Bound rollup admission and publication waits without canceling admitted cache work or removing ingest from rotation.
vishr
commented
Aug 28, 2026
vishr
commented
Aug 28, 2026
vishr
commented
Aug 28, 2026
vishr
commented
Aug 28, 2026
vishr
commented
Aug 28, 2026
vishr
commented
Aug 28, 2026
vishr
commented
Aug 28, 2026
vishr
commented
Aug 28, 2026
vishr
commented
Aug 28, 2026
vishr
commented
Aug 28, 2026
Five review rounds relocated the process-wide serialization point four times (parquetMu, writeGate, publishMu, back to parquetMu) without the finding rate falling, because each fix bolted a timeout or count onto a call graph where no constant could see the cost of the work it budgeted. This bounds that work at admission instead. The edge rollup stops after maxEdgeSubWindowsPerPass sub-windows, persists a resume cursor, and leaves the ingested watermark behind until the window finishes. A seed load or backfill compresses a wide start_time range into a narrow ingested window, so chunking ingested time alone let one pass expand into an unbounded number of sub-windows and hold the read gate for as long as that took. Cancelling mid-transaction only discarded the work and retried it forever, so the pass now yields instead, and hold time is a function of the constant rather than the shape of the data. PublishParquet budgets the reader drain and the directory swap separately. Sharing one deadline meant a publisher that spent most of it waiting entered the exclusive window with almost nothing left and cancelled its own renames after paying the full cost of excluding every reader. CommitBatch takes a context, threaded from the writer through Repository.Commit. It was the last publish-gate waiter using context.Background(), so a stalled publication blocked ingest indefinitely and OTLP clients lost rows to their own timeouts. A compaction marker that fails recovery maxCompactionRecoveryAttempts times is set aside as COMPACTION.json.failed, and Open sets it aside and boots rather than exiting. Recovery correctly gates retention, compaction, and retired cleanup, so without a give-up path one bad marker latched all maintenance off for the process lifetime and refusing to boot left no way to run the cleanup that would clear it. Nothing is deleted: the staged output survives and cleanupRetired now protects the .retired-<output> sets named by any live or set-aside marker. Cancelled passes do not count toward giving up. PruneBefore holds p.mu only to mutate the in-memory set, not across up to 64 renames and an fsync. CommitBatch's first action reads that same mutex, which made it a second undeadlined serialization point on the ingest path, invisible to the publish gate's accounting. Adopting a directory left by an interrupted commit now validates it before taking the gate rather than inside it. Removes Repository.Trace and the panicking parquetReadGate.RLock/Lock wrappers: both were duplicate paths, and Repository.Trace bypassed the read gate entirely. Adds invariant tests for the bounded rollup hold, the marker give-up and its rollback-set protection, booting past an unrecoverable marker, and ingest surviving a stalled publication, so a future relocation fails CI instead of waiting for another review round.
Setting a failed compaction marker aside was supposed to end the failure, but two paths kept it going. protectedRetiredSuffixes returned an error when a marker would not parse. The set-aside marker stays on disk by design, so cleanupRetired failed on it during every subsequent Open and the process exited on each boot — precisely the outcome that setting it aside exists to prevent, and worse than the latch it replaced because it survives restarts. An unreadable marker names no rollback set, so it now retains every retired directory and logs, deleting nothing. cleanupCompactionArtifacts decided whether to keep the staged output from a boolean set on the boot that performed the quarantine. On the next boot the live marker was gone, the flag was false, and the staged output was removed — leaving an operator able to roll the compaction back but never to complete it, though the doc comment promised both. It now keys off the presence of the set-aside marker, so the stage survives as long as the marker does. PublishReplacement held p.mu across the output move, up to 64 input renames, and an fsync. CommitBatch's first action reads that same mutex, so this was the ingest-blocking pattern PruneBefore's comment already documents as forbidden; p.mu now covers only the in-memory map mutations. CommitBatch no longer leaks its staging directory when the publish rename loses to an existing final directory. selectCompactionBatches applies the same MaxIngestedNanos > 0 guard as the counting loop, so a batch with no ingest timestamp cannot be pulled into a group it was never counted in. Adds regression tests for repeated boots against an unreadable marker and for the staged output surviving every boot the marker survives.
Keep compaction recovery fail-closed instead of bypassing a live marker. Bound only publish admission and edge-rollup work, and read stats under a stable Parquet namespace.
Review of a70592e, reading it directly rather than delegating. updateParquetStats now takes the Parquet read gate, which is the right call — it was the one reader that could observe a namespace mid-swap. But it runs on the rollup and maintenance loop goroutines under the process context, so unlike every other reader it had no deadline, and a stuck publication would park both loops indefinitely. The wait is now bounded; a skipped refresh costs one tick of stale gauges, which is strictly less than what waiting costs. validateCompactionID duplicated telemetry.validateBatchID's grammar in a second package, where the two had already begun to diverge in implementation. The rule decides which strings may become directory names under the storage root, so two copies means two gates that can disagree about what is safe. The telemetry version is now exported and the store calls it. cleanupRetired names the coupling it acts on: a live marker is the only thing that makes a retired directory unreclaimable, so deleting COMPACTION.json without first renaming its <id>.retired-<output id> directories back to <id>.batch is not a rollback — it makes those rows deletable, and this loop removes them on the next pass. The startup log warns about this; the code that does the deleting did not say so. Also documents the site-level runbook changes from the review: the troubleshooting section for a startup-blocking marker, and the data layout reference, which still described COMPACTION.json as a transient marker rather than a startup gate. Adds a regression test for the bounded stats refresh.
Remove both possible replacement locations during manual rollback so restored inputs cannot coexist with a published output. Exercise the stats reader's own wait limit without caller cancellation.
aacffe0 corrected the rollback guidance to delete both possible replacement locations, since recovery checks for the output either staged under compaction/ or already published as <output id>.batch. The docs and the rollback field were updated; the two log fields that describe where things are were not. An operator reading the startup log was told the staged replacement was intact and pointed at compaction/. When the interruption happened after publication that directory is empty, and the reachable conclusion from "the log says it is here and it is not" is that the merged rows are gone — which is the panic that ends in rm -rf of the data directory, the one outcome failing closed exists to prevent. The field now names both locations, matching the rollback procedure and the troubleshooting guide, and staged_replacement is renamed to compaction_staging so the key stops asserting a location the code does not guarantee.
A failed publish may have already removed some retired inputs. Require operators to verify the complete rollback set before deleting the replacement or marker.
238f2fe dropped the claim that the retired inputs are all intact, because a publish that failed partway may already have removed some, and added a precondition to the startup log: verify every marker input exists as either <id>.batch or <id>.retired-<output id> before deleting anything. The troubleshooting guide was not updated with it and still made the stronger claim. That left the guide's procedure unsafe in exactly the case the log now warns about. Its step 2 covered ids present as <id>.retired-<output id> and ids present as <id>.batch, but said nothing about an id present as neither — the dangerous one, where the rows survive only inside the replacement. An operator with a partially removed rollback set would have followed the steps in order, deleted the replacement, and destroyed those rows permanently. The guide now states that the rollback set may be incomplete, verifies it as an explicit step before any deletion, and tells the operator to stop and either restore from backup or complete the compaction instead. The caution covers both ordering constraints rather than only the second.
An input present in both active and retired forms is ambiguous and cannot be safely renamed. Require exactly one copy before deleting the compacted replacement or marker.
The manual rollback lived in both logUnresolvedCompaction and the troubleshooting guide, and every revision since it was written corrected one copy and left the other stating something the code no longer guarantees: b8345fb guide named both replacement locations; the log did not 238f2fe log dropped the "all intact" claim and added a verification precondition; the guide kept the claim and the old procedure 7e4ab8c guide caught up 33f3cb1 guide required each input in exactly one form; the log still said either form, which permits the ambiguous case it was added to reject Each copy was correct when written. Operator guidance that is wrong in one of the two places an operator might read is worse than guidance in one place they have to open, so the log now links the runbook and states only what it alone knows: the absolute paths on this machine, that nothing has been cleaned up, that a plain retry is the first move, and the one invariant that must not be got wrong offline — deleting the marker by itself is not a rollback. The link is checked by a test that resolves its anchor against the guide's headings, so the pointer cannot rot the way the copy drifted. Also rewraps the paragraph 33f3cb1 left broken mid-sentence.
Parallelize group commit and native compaction while preserving atomic publication and bounded query memory. Add deep offline verification and explicit quarantine for unreadable authoritative batches.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The exploratory storage comparisons and benchmark POCs remain local under the ignored
experiments/directory.Breaking change
Existing DuckLake, WAL, and segment telemetry is not migrated. Deployments must start with a clean
storage.data_dir. No compatibility or fallback storage path is included.Verification
just checkand pre-push gate