feat(duckdbservice): export ducklake commit-loop stats from ducklake_commit_stats() - #1012
Conversation
Test Impact PlanDeterministic summary of how this PR changes tests, CI runners, and coverage-risk signals. Summary
Signals
Coverage risk: neutral or increased No coverage-reduction warnings detected. |
…commit_stats()
The posthog ducklake extension is gaining a ducklake_commit_stats() table
function exposing process-global cumulative counters from its internal
commit retry loop. Workers now poll it every 30s and re-export the counters
as duckgres_worker_ducklake_commit_* Prometheus metrics (labelled by
catalog; conflicts additionally by cause). Unlike the existing
duckgres_worker_ducklake_conflict_* counters, which only see conflicts that
escape the extension's retry loop, these see inside it.
Contract (shared with the extension): no arguments; columns catalog
VARCHAR, stat VARCHAR, value BIGINT; one row per (catalog, stat); values
cumulative and monotonic per process. Stats: attempts, successes,
retries_exhausted, nonretryable_errors, backoff_ms, total_commit_ms, and
conflicts.{primary_key,unique,conflict,concurrent}.
The poller (commitStatsLoop, same lifecycle as metadataMetricsLoop: started
in NewDuckDBService, stops on pool close) probes duckdb_functions() once
per DuckDB engine; when the function is absent — true for every
currently-released extension binary — it logs once at info and disables
polling, so everything degrades gracefully with the pinned extension.
Cumulative values are converted to counter increments via last-seen delta
tracking; a value that goes down (contract violation — defensive only) is
treated as a fresh baseline. Scrapes prefer the worker's controlDB side
connection so they never queue behind a long-running user query.
No e2e-mw-dev harness assertion: no released extension binary provides the
function yet, so the positive path cannot be exercised against the real
cluster; the graceful-degradation path and the delta/mapping logic are
covered by unit tests against real in-memory DuckDB (a table macro stands
in for the extension's table function).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LUnQHp46rmLKdupmDUr7kG
9c8b162 to
be2e015
Compare
fuziontech
left a comment
There was a problem hiding this comment.
Adversarial self-review (requested by James). One real bug found and fixed in be2e015; the rest verified clean.
Bug found & fixed: the delta tracker's defensive path for negative values reset the baseline to 0 (lastSeen[key] = 0), which would make the next valid cumulative value count in full — double-exporting everything already recorded (100 exported, glitch, 102 arrives → +102 instead of +2). The inline comment claimed this prevented inflated deltas; it caused them. Fix: keep the baseline untouched and emit nothing. The test now regression-covers the exact double-count scenario ("post-glitch value counts only the increase over the kept baseline"). Contract-wise negatives can't occur (extension counters are monotonic int64 atomics), so this was latent, but a defense that amplifies is worse than none.
Verified correct:
- Probe caching is by
*sql.DBidentity and a probe error records no verdict (retries next tick) — no risk of permanently wedging into "unavailable" from a transient failure; engine replacement triggers exactly one re-probe. - Exactly one poller goroutine per process:
NewDuckDBServiceis invoked once in the service path, and the exporter state is confined to that goroutine (no locking needed, as documented). scrapeCommitStatstakesp.mu.RLockonly to read thecontrolDBpointer, never across a query; 5s scrape timeout; falls back to activation DB only whencontrolDBis unwired (test paths).- Counter-reset semantics: decrease → current value as delta is the standard restart heuristic; correct here since exporter state and extension counters share process lifetime.
- Zero-delta touches materialize all known series on first poll (good for
rate()continuity).
Notes for reviewers (accepted trade-offs):
*_ms_totaldeviates from the Prometheus base-unit (seconds) convention — kept deliberately for 1:1 symmetry with the extension contract and integer delta math.rate()dashboards just divide by 1e3.- Unknown
conflicts.<cause>values export under their own label (forward-compatible with new extension causes; cardinality is extension-controlled — ours). Unknown non-conflict stats are dropped. - The positive e2e path can't run until an extension binary ships
ducklake_commit_stats()(PostHog/ducklake#35 → next release tag); until then prod exercises exactly the probe-absent path, which is unit-tested against real DuckDB.
Post-fix: go build, go vet, unit tests, just lint all clean.
Summary
Workers now poll the ducklake extension's (upcoming)
ducklake_commit_stats()table function every 30s and re-export its process-global counters as Prometheus metrics. This gives visibility INSIDE the extension's commit retry loop — the existingduckgres_worker_ducklake_conflict_*counters only see conflicts that escape it, which is why they read ~0 while megaduck's metadata store shows a 12.4% transaction rollback rate.New metrics (all labelled by
catalog; conflicts additionally bycause∈ {primary_key, unique, conflict, concurrent}):duckgres_worker_ducklake_commit_attempts_totalduckgres_worker_ducklake_commit_successes_totalduckgres_worker_ducklake_commit_retries_exhausted_totalduckgres_worker_ducklake_commit_nonretryable_errors_totalduckgres_worker_ducklake_commit_backoff_ms_totalduckgres_worker_ducklake_commit_duration_ms_totalduckgres_worker_ducklake_commit_conflicts_totalDesign
ducklake_commit_stats()→ rows of(catalog VARCHAR, stat VARCHAR, value BIGINT), cumulative and monotonic per process. Cumulative→counter conversion via last-seen delta tracking (decrease → treated as fresh baseline; negative values → dropped, never stored as baseline).duckdb_functions()) once per engine; no released extension binary has the function yet, so on current pins this logs once at info and stays dormant. Ships safely before/independently of the extension PR.commitStatsLoopmirrorsmetadataMetricsLoop— started inNewDuckDBService, 30s ticker, stops via the pool'sstopCh, scrapes overcontrolDBto avoid queueing behind pinned user-query connections.Test plan
prometheus/testutil, probe against real in-memory DuckDB (macro stand-in), probe-once/disabled path, nil-DB skipjust test-unit(incl. repository hygiene),just lint,go vet: cleanducklake_commit_stats()(noted per harness rule); degradation path is what current pins exerciseFollow-ups: millpond needs the same poller (separate repo — that's the writer where megaduck's numbers will appear); Grafana panels once metrics flow.
🤖 Generated with Claude Code
https://claude.ai/code/session_01LUnQHp46rmLKdupmDUr7kG