Repository navigation
perf(telemetry): cut commit allocation; fix(query): cancelled reads poisoning the pool; pin DuckDB v2.0.0-alpha46057 - #310
Merged
Merged
Conversation
Move the embedded engine from v2.0.0-alpha43763 (96063b9e39) to v2.0.0-alpha46057 (b15358c371), the newest completed DuckDB 2 build. The duckdb-v2.0.0-alpha46057 release mirrors DuckDB's staging archives byte for byte, and the wrapper pins their SHA-256 values. The engine adds one built-in, read_single_parquet_file. It reads files, so the telemetry SQL boundary blocks it like read_parquet, and the function snapshot is regenerated. No built-in was removed or retyped, and no code change was needed. SELECT version() reports v2.0.0-alpha46057.
Fanout uses Dependabot for dependency updates; Renovate is not configured, so its bot no longer needs a CLA exemption.
Every new zstd encoder at the library's fastest level allocated 8 MiB of match history, and the garbage collector empties the encoder pool, so that history was most of what a commit allocated. Parquet pages are compressed with a 1 MiB window instead, which covers a whole page. A 50,000-span commit now allocates 116-121 MB instead of 369-383 MB and takes 293-322 ms instead of 472-509 ms; the allocation ratchet drops from 7.0x to 3.0x. - Buffer trace-index entries instead of one write call per trace. - Measure a request's bytes once at submission rather than on admission, grouping and release. - Build each telemetry Parquet schema once per row type. - Dictionary-encode span kind and status, which come from fixed OTLP enums; client-supplied names stay plain. Bound every dictionary column at 1 MiB, after which it continues in plain pages. - Explain an empty panel with one scan that counts every omitted-filter rerun, falling back to separate counts when a predicate fails.
A read transaction begun with the request context was rolled back by database/sql on its own goroutine when that context was cancelled, as a dashboard refresh does to the requests it supersedes. DuckDB could deliver the cancelled statement's interrupt to that ROLLBACK, so the transaction stayed open and the pooled connection failed every later read with "Current transaction is aborted" or "cannot start a transaction within a transaction". Cancelling reads in a loop poisoned the pool in 4 of 7 runs on DuckDB v2.0.0-alpha46057 and 1 of 7 on alpha43763. Read transactions now begin with a context that cannot be cancelled and are rolled back by their owner; statements inside still run under the request context. A connection whose BEGIN or ROLLBACK fails is discarded instead of returned to the pool. The same loop then passed 6 of 6 runs.
vishr
changed the title
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
1 participant
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.
What changed
Four commits, intended to be rebase-merged:
build(duckdb): pin DuckDBv2.0.0-alpha46057(upstreamb15358c371). The new built-inread_single_parquet_filereads files, so the telemetry SQL boundary blocks it, with rejection tests and a regenerated function snapshot.SELECT version()returnsv2.0.0-alpha46057; the releaseduckdb-v2.0.0-alpha46057mirrors the upstream archives and checksums.chore(ci): droprenovate[bot]from the CLA allowlist.perf(telemetry): Parquet pages are compressed with a 1 MiB zstd window. At the library's default 4 MiB window every new encoder allocated 8 MiB of match history, and that history was most of what a commit allocated. A 50,000-span commit now allocates 116–121 MB instead of 369–383 MB and takes 293–322 ms instead of 472–509 ms (BenchmarkCommitBatch, three runs each, measured back to back), and the allocation ratchet drops from 7.0x to 3.0x. The commit also buffers trace-index writes, measures request bytes once, caches each Parquet schema, dictionary-encodes span kind and status, bounds every dictionary column at 1 MiB, and explains an empty panel with one scan.fix(query): keep cancelled reads from poisoning pooled connections. A read transaction begun with the request context was rolled back bydatabase/sqlon another goroutine when the context was cancelled, as a dashboard refresh does to superseded requests; DuckDB could deliver the cancelled statement's interrupt to thatROLLBACK, leaving the transaction open and the pooled connection failing every later read ("Current transaction is aborted", "cannot start a transaction within a transaction"). This surfaced as a failed browser smoke run on this PR. Cancelling reads in a loop poisoned the pool in 4 of 7 runs on the new engine and 1 of 7 on the old one; with the fix, 6 of 6 passed. Read transactions now begin with a non-cancellable context and are rolled back by their owner, and a connection whoseBEGINorROLLBACKfails is discarded.The file format and physical schema are unchanged: encodings are page layout, and existing batches stay readable and mergeable. Client-supplied span names (namespace, service, operation) stay plain. No configuration, API or migration changes.
Verification
just check(pre-push hook)just test-race(ingest and query paths changed)Read-path speedups are not claimed here: they have not been measured on this final code.