Skip to content

Replace DuckLake with authoritative Parquet storage - #207

Merged
vishr merged 31 commits into
mainfrom
feat/telemetry-segment-parquet
Aug 28, 2026
Merged

Replace DuckLake with authoritative Parquet storage#207
vishr merged 31 commits into
mainfrom
feat/telemetry-segment-parquet

Conversation

@vishr

@vishr vishr commented Aug 26, 2026

Copy link
Copy Markdown
Member

Summary

  • replace DuckLake, WAL, and segment storage with atomic Parquet batch directories
  • use the filesystem as the telemetry catalog and keep SQLite for control-plane state only
  • write spans, logs, and metrics concurrently through bounded durable commit workers
  • serve trace lookups through persistent on-disk indexes while DuckDB handles broad scans and rebuildable rollups
  • compact and retain Parquet with crash recovery, bounded query memory, and immediate commit-failure propagation
  • force every pooled DuckDB connection to UTC so rollup windows remain correct in non-UTC deployments

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

  • production binary first boot, admin setup, authenticated OTLP ingest, Parquet queries, compaction, graceful shutdown, restart, persisted queries, and post-restart ingest
  • 376 rows accepted with zero drops in the initial E2E pass; rollup totals increased after restart
  • full local just check and pre-push gate
  • full Go race suite plus repeated storage, query, and UTC regression tests
  • Go and UI dependency audits, generated notices, UI builds/tests, documentation generation, and Astro site build
  • storage benchmarks: approximately 988k spans/s through the production four-worker writer and approximately 1.01 ms median indexed trace lookup on the test host

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.
Comment thread internal/telemetry/store/compaction.go Outdated
Comment thread internal/telemetry/segment/span_store.go Outdated
Comment thread internal/observability/logs.go Outdated
Comment thread internal/telemetry/store/writer.go Outdated
Comment thread internal/telemetry/store/repository.go Outdated
Comment thread internal/observability/trace.go Outdated
Comment thread internal/query/duck.go Outdated
Comment thread internal/api/health.go
Fail fast on legacy DuckLake data instead of hiding it. Drain and publish compaction without blocking reads or commits, and surface failed writes immediately.
Comment thread internal/telemetry/store/writer.go Outdated
Comment thread internal/telemetry/store/repository.go Outdated
Comment thread internal/telemetry/store/compaction.go Outdated
Comment thread internal/query/duck.go Outdated
Comment thread internal/telemetry/segment/signal_store.go Outdated
Comment thread internal/telemetry/store/repository.go Outdated
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.
Comment thread internal/telemetry/store/compaction.go Outdated
Comment thread internal/telemetry/store/repository.go Outdated
Comment thread internal/telemetry/store/writer.go Outdated
Comment thread internal/observability/logs.go Outdated
Comment thread internal/query/duck.go Outdated
Comment thread internal/query/duck.go
Comment thread internal/observability/trace.go Outdated
Comment thread internal/query/duck.go Outdated
vishr added 2 commits August 26, 2026 15:31
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.
Comment thread internal/observability/logs.go Outdated
Comment thread internal/observability/trace.go Outdated
Comment thread internal/query/duck.go
Comment thread internal/query/duck.go
Comment thread internal/telemetry/parquet.go Outdated
Comment thread internal/telemetry/parquet.go Outdated
Comment thread internal/telemetry/parquet.go
Comment thread internal/query/duck.go Outdated
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.
Comment thread internal/query/duck.go
Comment thread internal/query/duck.go
Comment thread internal/telemetry/store/repository.go
Comment thread internal/query/duck.go Outdated
Comment thread internal/telemetry/parquet.go Outdated
Comment thread internal/telemetry/parquet.go Outdated
Comment thread internal/telemetry/parquet.go Outdated
Comment thread internal/query/duck.go
Comment thread internal/telemetry/store/compaction.go
Comment thread internal/telemetry/store/repository.go
vishr added 12 commits August 27, 2026 20:10
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.
@vishr
vishr merged commit c3dc7d9 into main Aug 28, 2026
8 checks passed
@vishr
vishr deleted the feat/telemetry-segment-parquet branch August 28, 2026 17:56
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.

1 participant