Skip to content

Replace DuckLake with authoritative Parquet storage - #207

Open
vishr wants to merge 19 commits into
mainfrom
feat/telemetry-segment-parquet
Open

Replace DuckLake with authoritative Parquet storage#207
vishr wants to merge 19 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/telemetry/parquet.go Outdated
Comment thread internal/query/duck.go
Comment thread internal/telemetry/store/repository.go Outdated
Comment thread internal/telemetry/parquet.go Outdated
Comment thread internal/telemetry/store/compaction.go Outdated
Comment thread internal/telemetry/parquet.go Outdated
Comment thread internal/telemetry/parquet.go
Comment thread internal/telemetry/parquet.go
Comment thread internal/query/parquet_gate.go Outdated
Acquire reader exclusion before the storage publication lock and cap each maintenance pass so rollups cannot stall ingest. Keep recovery namespace swaps reader-atomic and escalate sustained maintenance failures.
Comment thread internal/api/health.go Outdated
Comment thread internal/query/duck.go Outdated
Comment thread internal/query/duck.go Outdated
Comment thread internal/query/duck.go Outdated
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
return err
func (d *Duck) lockRollupParquetRead(ctx context.Context) error {
waitCtx, cancel := context.WithTimeout(ctx, rollupReaderLease)
defer cancel()

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HIGH — unbounded rollup snapshot hold vs. a fixed 60s publisher budget (new in fbc56044).

lockRollupParquetRead now bounds only admission (15s); once the reader is admitted the rollup chunk runs under the process context with no deadline. In the same commit rollupChunkNanos went back to 1h (duck.go:903), and refreshEdgeRollup loops over every edgeStartChunkNanos (30 min) sub-window of the affected start_time range inside one transaction while holding parquetMu.RLock.

Meanwhile PublishParquet (duck.go:1462) still caps the publisher at 2*defaultWriterGrace = 60s.

Concrete failure: any instance restarting with a backlog (or after an outage) processes a 1h ingest chunk covering up to ~115M rows at the certified ~32k rows/s ceiling; the edge rollup's sub-window loop over a backfill whose start_time spans days runs hundreds of self-joins in one transaction. That single reader holds the snapshot for minutes. Every PublishParquet queued during it hits the 60s deadline, so retention and compaction make zero progress for the entire catch-up — the storage-growth spiral, now reported only as degraded + HTTP 200.

This is the fifth relocation of the same serialization point rather than a fix: round 5 canceled the rollup at 15s (rollups never committed); round 6 removed the cancel (maintenance never runs). The reader hold and the publisher budget have to be derived from one another, not chosen independently.

Comment thread internal/query/duck.go
compactStart := time.Now()
var cleanupErr, parquetErr, compactErr error
compacted := 0
if recoveryErr == nil {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HIGH — recoveryErr gates cleanup + prune + compaction, and nothing clears the marker (carried from 526fd87c/895c238a, still open).

if recoveryErr == nil guards CleanupParquet, PruneParquetPass and CompactParquetPass. RecoverParquet only does work when COMPACTION.json exists, and it fails whenever completeCompaction -> PublishReplacement -> PublishParquet times out.

Combined with the reader-hold issue above this latches permanently: a compaction interrupted by the 60s publish deadline leaves the marker; every subsequent hourly pass re-enters RecoverParquet, hits the same long rollup reader, fails again, and skips retention/compaction/cleanup entirely. maintenanceFailures climbs forever behind a 200. There is no path that abandons or quarantines a marker that cannot be completed.

return nil, err
}
r := &Repository{root: root, Parquet: parquetStore}
if err := r.recoverCompaction(context.Background(), func(ctx context.Context, publish func(context.Context) error) error { return publish(ctx) }); err != nil {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HIGH — an unloadable compaction stage makes the process unbootable (carried from 526fd87c).

Open runs recoverCompaction before cleanupCompactionArtifacts. If a staged compaction output exists but cannot be loaded — truncated spans.parquet, a torn trace.fidx, a metadata version this binary doesn't accept — recoverCompaction takes the stageExists branch, PublishReplacement's loadStoredBatch(source) fails, Open returns an error and cmd/fanout/main.go calls os.Exit(1).

Restarting repeats it: the os.RemoveAll(root/compaction) that would clear the bad stage is unreachable because it runs after recovery. Recovery is a manual rm -rf on an observability appliance whose value is being up during an incident. The same class applies to readBatchMetadata's strict Version != 2 + fatal loadBatches: one bad .batch directory and the store will not open.

Comment thread internal/query/duck.go
return false
// PublishParquet limits reader exclusion to the atomic directory swap.
func (d *Duck) PublishParquet(ctx context.Context, publish func(context.Context) error) error {
publishCtx, cancelPublish := context.WithTimeout(ctx, 2*defaultWriterGrace)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MEDIUM — publishCtx budgets the reader drain and the swap out of the same 60s (new in fbc56044).

publishCtx is created before parquetMu.LockContext(publishCtx) and then passed straight into publish(publishCtx). If the reader drain consumes 55s, the closure — which starts with p.lockPublish(ctx) — receives a context with ~5s (or already expired) and can fail immediately even though publishGate is free. The exclusive parquetMu hold is then spent for nothing and the whole maintenance pass errors, feeding the latch above.

The swap's budget should start when the lock is acquired (a fresh context.WithTimeout after LockContext succeeds), not be whatever is left over from the wait.

prepared.traces.path = filepath.Join(final, "trace.fidx")
}

if err := p.lockPublish(context.Background()); err != nil {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MEDIUM — the ingest hot path is the only publishGate waiter with no deadline (fbc56044).

CommitBatch calls p.lockPublish(context.Background()) here (and at line 158). Every other acquirer — PruneBefore, PublishReplacement, RestoreRetiredInputs — now passes a real context, so the one participant that can convert a stall into permanent data loss is the one that waits forever.

publishGate is held by maintenance publications across up to 64 os.Rename calls plus a syncDirectory of a directory that grows with the batch count. On a degraded disk that hold is unbounded, Submit blocks, the OTLP client times out and gives up, and the rows are gone. CommitBatch should take the submitting request's context (or a bounded one) so the writer can surface backpressure instead of hanging.

return err
}
defer p.unlockPublish()
p.mu.Lock()

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MEDIUM — p.mu (the batches-map lock) is held across up to 64 renames plus an fsync, and the ingest path takes it (carried from 895c238a).

PruneBefore takes p.mu.Lock() for the whole retire loop; PublishReplacement (line ~722) does the same. CommitBatch's very first action is p.hasBatch(metadata.ID) -> p.mu.RLock().

So ingest commits block on maintenance filesystem work through a second, undeadlined serialization point that is invisible to the publishGate/parquetMu accounting. syncDirectory(p.batchesDir) inside that window costs more as the directory grows, which is exactly the state the pass is trying to fix. The map mutation should be separated from the filesystem work (do the renames, then take p.mu only to swap map entries).

return nil
}
if info, err := os.Stat(final); err == nil && info.IsDir() {
if err := p.lockPublish(context.Background()); err != nil {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MEDIUM — the one path where validation was not hoisted out of the critical section (fbc56044).

The commit message says validation moved outside the publish lock, and on the main path it did (loadStoredBatch(stage) now runs before lockPublish). But this branch — and the rename-failed branch at line 260 — still call registerBatch(final) inside the gate, and registerBatch -> loadStoredBatch opens every signal's Parquet footer and walks the entire trace.fidx validating strict ordering and contiguity. For a compacted batch that is a full scan of a 3.2M-entry index while the ingest path is blocked behind the gate.

Either hoist these loads the way the main path was, or drop the branch: with p.batches populated from disk at open and prune/compaction renaming directories away, final existing while hasBatch is false is not reachable at runtime.

Comment thread internal/query/duck.go
EndpointDisabledStateKey = "endpoint_rollup_v1_disabled"
defaultDuckDBPoolSize = 1
parquetMaintenanceBatchLimit = 64
parquetMaintenancePhaseBudget = 10 * time.Minute

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MEDIUM — the phase budget does not bound the phase, and the reclaim rate is still below the creation rate.

Replacing parquetMaintenancePublishLimit = 4 with a 10-minute budget fixes retention (a prune publication retires 64 directories cheaply and the loop can run thousands of times). It does not fix compaction, and the two constants are not mutually consistent:

  • The budget is only checked between CompactParquet calls (compaction.go:180), so one unbounded DuckDB COPY over 64x50k rows plus PrepareReplacement's full re-read of that output can outrun the 1h MaintenanceInterval on its own. "10 minute budget" names something the code does not enforce.
  • Each publication reclaims at most 64 directories and costs a write and a read of up to 3.2M rows. In a 10-minute window that is on the order of 10^1-10^2 publications, i.e. low thousands of directories per hour, against a creation rate of one directory per OTLP export request that enqueueSubmissions could not group (at 32k rows/s with typical batch sizes that is 10^3-10^5 per hour).

Everything scales with that directory count: DuckDB re-expands the *.batch glob and opens every footer on every query, updateParquetStats stats 3 files per batch on every rollup tick (60s), Trace opens every batch's .fidx, and CommitBatch fsyncs batchesDir on the ingest path. The cap moved from a count to a clock but the ceiling is still below the floor.

for {
count, err := r.CompactParquet(ctx, compactor, maxBatches)
total += count
if err != nil || count == 0 || !time.Now().Before(deadline) {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MEDIUM — budget checked only after a completed compaction, with no bound on a single one.

The intent ("slow compactions finish instead of being canceled and retried forever") is right, but the result is that the storage phase has no upper bound at all: ctx is the process context, MergeParquet is an unbounded COPY, and PrepareReplacement re-reads the entire output. A pathological group can hold the maintenance goroutine past the next hourly tick.

The missing piece is an admission decision rather than a cancellation: size the group from the estimated row count so a single compaction is expected to fit the remaining budget, then let it run to completion uncancelled. As written, budget and maxBatches are two constants that cannot see each other's cost.

var cleanupErr error
for _, entry := range entries {
name := entry.Name()
if !entry.IsDir() || strings.HasSuffix(name, telemetry.BatchSuffix) || !strings.Contains(name, ".retired") {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LOW-MEDIUM — retired-directory cleanup depends on a caller ordering the type cannot enforce.

Moving cleanup into Repository is the right ownership call. But cleanupRetired deletes any directory whose name merely contains .retired, including the <id>.retired-<output> directories that RestoreRetiredInputs needs to roll a compaction back. The only thing keeping that safe is that runRepositoryMaintenance happens to call RecoverParquet before CleanupParquet — and CleanupParquet is exported, so nothing stops a future caller from inverting it and destroying the rollback set for a live marker.

Make the invariant structural: read COMPACTION.json inside cleanupRetired and skip any suffix matching a live marker's output ID, rather than documenting the obligation in a comment.

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