Repository navigation
Drop DuckDB file scan state outside the multi-file lock - #10486
Merged
Merged
Conversation
Merging this PR will degrade performance by 10.04%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | WallTime | threshold_avx512[8] |
1.8 µs | 2 µs | -10.04% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing rk/duckdb-background-reader-drop (2272e44) with develop (7319276)
joseph-isaacs
added
the
action/bench-sql
github-actions
Bot
removed
the
action/bench-sql
Contributor
Polar Signals Profiling ResultsLatest Run
Powered by Polar Signals Cloud |
Contributor
Benchmarks: PolarSignals Profiling 📖Commits: PR datafusion / vortex-file-compressed / ns (0.998x ➖, 0↑ 0↓)
No file size changes detected. |
Contributor
Benchmarks: FineWeb NVMe 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.002x ➖, 1↑ 0↓)
datafusion / parquet / ns (0.998x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (1.010x ➖, 4↑ 1↓)
duckdb / parquet / ns (0.997x ➖, 0↑ 0↓)
No file size changes detected. |
Contributor
Benchmarks: TPC-H SF=1 on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.000x ➖, 0↑ 0↓)
datafusion / parquet / ns (0.999x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (1.000x ➖, 0↑ 1↓)
duckdb / parquet / ns (0.999x ➖, 0↑ 0↓)
No file size changes detected. |
Contributor
Benchmarks: Clickbench on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.005x ➖, 0↑ 1↓)
datafusion / parquet / ns (0.995x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (0.839x ✅, 22↑ 0↓)
duckdb / parquet / ns (1.005x ➖, 1↑ 1↓)
No file size changes detected. |
Contributor
Benchmarks: TPC-DS SF=1 on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.999x ➖, 0↑ 1↓)
datafusion / parquet / ns (1.000x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (0.990x ➖, 4↑ 1↓)
duckdb / parquet / ns (1.002x ➖, 0↑ 4↓)
No file size changes detected. |
Contributor
Benchmarks: TPC-H SF=10 on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.992x ➖, 0↑ 0↓)
datafusion / parquet / ns (0.999x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (0.987x ➖, 0↑ 0↓)
duckdb / parquet / ns (0.995x ➖, 0↑ 0↓)
No file size changes detected. |
Contributor
Benchmarks: TPC-H SF=1 on S3 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.052x ➖, 0↑ 4↓)
datafusion / parquet / ns (0.860x ➖, 1↑ 0↓)
duckdb / vortex-file-compressed / ns (0.916x ➖, 1↑ 0↓)
duckdb / parquet / ns (0.894x ➖, 0↑ 0↓)
|
DuckDB releases its last reference to a file reader while it holds the global lock of the multi-file scan (`TryInitializeNextBatch`): either when the file runs out of work, or when a thread moves to another file. Dropping a Vortex `OpenFileReader` tears down its layout reader tree and conversion cache, which took about 0.8 ms per file on a 192-core machine. Every other scan thread waited for the lock during that time. Move the layout reader and conversion cache into an `Arc<FileScanState>` that the local states scanning the file share. The callbacks that DuckDB calls under the lock only move these references: - When a thread starts a split of another file, its reference to the previous file goes to its list of retired files. - When a file has no splits left, the file gives its own reference to the local state of the calling thread. `reader_scan`, which runs without the lock, drops the retired files, and the remaining ones drop with the local state. A file that pruning skips drops its scan state in `reader_initialize`, under the file lock only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019VbXEWQzqmW5Sxb73DN1Zx Signed-off-by: Robert Kruszewski <github@robertk.io>
robert3005
force-pushed
the
rk/duckdb-background-reader-drop
branch
from
October 14, 2026 13:22
d99c47a to
2272e44
Compare
robert3005
changed the title
joseph-isaacs
approved these changes
Oct 14, 2026
joseph-isaacs
pushed a commit
that referenced
this pull request
Oct 14, 2026
Resolves the DuckDB file reader conflict with #10486: the open file keeps develop's shared FileScanState and adds the VortexFile and prepared-skip state the V2 scan and file prefetch use. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P1oQtzTgSzkvrGDKisoNko Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk> Signed-off-by: Claude <noreply@anthropic.com>
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
2 participants
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.
DuckDB releases its last reference to a file reader while holding the global multi-file scan lock, so dropping a Vortex file reader stalled every scan thread. The layout reader and conversion cache now live in an
Arcshared by the local states that scan the file; under the lock we only move these references, and the local state drops them outside it.ClickBench on c8g.metal-48xl, with #10485 and the pooled buffer allocator, median ms:
🤖 Generated with Claude Code
https://claude.ai/code/session_019VbXEWQzqmW5Sxb73DN1Zx