Fix #933: cap main DuckDB memory_limit, per-pair compaction exclude detection#955
Merged
erikdarlingdata merged 2 commits intoMay 15, 2026
Merged
Conversation
CompactParquetFiles detected CompactionExcludeColumns once, globally, across the union schema of every source file in a group. It then applied that "* EXCLUDE (col)" clause to each pair in the pairwise merge. query_plan_text was added to query_store_stats in migration v13 (2026-02-23). A reporter's archive contains both pre-v13 files (no column) and post-v13 files (column present). The global DESCRIBE saw the column in the newer files, so every merge step ran with "* EXCLUDE (query_plan_text)" — including the steps that merged two pre-v13 files, which fail with: Binder Error: Column "query_plan_text" in EXCLUDE list not found in FROM clause Extract the schema detection into BuildSelectClause(table, paths) and call it per merge-set instead of once globally — with the actual pair in the pairwise path, and with all sources in the small-group path. A pair that doesn't carry an exclude-column now merges with a plain "*". Verified against DuckDB CLI v1.5.2: DESCRIBE of an [old, old] pair correctly omits the column, and "* EXCLUDE (query_plan_text)" on that pair reproduces the reporter's exact Binder Error. Cost is one extra DESCRIBE per merge step — parquet footer reads, not data. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
#933's titled complaint is "Memory usage on client": Lite holds ~2.7-2.9 GB after 10 minutes with 4 servers. The compaction OOMs everyone has been chasing in this thread are a downstream symptom — by the time compaction runs the app already holds 2.7 GB, leaving little headroom on the reporter's 16 GB / ~1.6 GB-free machine. Root cause: the main DuckDB ConnectionString set no memory_limit, so the buffer pool ran at the DuckDB default of 80% of system RAM (~12.8 GB on a 16 GB box). With archive parquet files accumulating on disk, every UI query over an archive view caches pages and the buffer pool grows freely. The fix has to navigate one wrinkle: parquet COPY in DuckDB v1.5.2 hits a buffer-manager-bypass pre-reservation that needs ~2-4 GB headroom. Capping the main connection at 1 GB statically would break ExportToParquet and the two COPY paths in ArchiveAllAndResetAsync. So: - ConnectionString: memory_limit=1GB (caps resting buffer pool — addresses the actual complaint by stopping the archive-page cache from growing unbounded). - Around each parquet COPY on the main connection: SET memory_limit='4GB', run the COPY, SET back to '1GB'. Factored into a WithRaisedCopyMemoryLimit helper so the three call sites stay consistent (ExportToParquet, and the two COPYs in ArchiveAllAndResetAsync). - Compaction connections (separate :memory: instances) keep their 4 GB cap from #952. Verified against DuckDB CLI v1.5.2 with synthetic query_snapshots-shaped data: - COPY table→parquet at 256MB/512MB/1GB: OOMs (pre-reservation, matches the read_parquet→parquet path we saw in #952 testing). - COPY table→parquet at 2GB/4GB: succeeds, peak RSS well under cap. - INSERT (Appender) and SELECT (including GROUP BY across 11k rows) work fine at 256MB cap — confirms collectors and UI queries don't have the pre-reservation behavior and aren't affected by the resting cap. Tradeoff: the resting cap forces buffer-pool eviction of cached archive parquet pages. Long-range historical UI queries that re-scan many parquet files will do more disk I/O. Live/recent-data queries against the hot DB are unaffected (hot DB is small enough to fit in 1 GB easily). Plus the per-merge-step BuildSelectClause from the previous commit fixes the separate query_store_stats Binder Error on archives that span the v13 schema change. Co-Authored-By: Claude Opus 4.7 (1M context) <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
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.
Summary
Two related fixes for #933, on top of #952:
memory_limitat 1 GB (raised transiently to 4 GB around parquet COPY operations).Binder Erroronquery_store_statsarchives that span the v13 schema change.Why this is the right fix
We re-read #933 carefully and realized the titled complaint is "Memory usage on client" — the reporter says Lite uses 2.7-2.9 GB after 10 minutes with 4 servers. The compaction OOMs we've been chasing are downstream symptoms: by the time compaction runs, the app already holds 2.7 GB, leaving little headroom on a 16 GB / 1.6 GB-free machine. The compaction tunings in #942 / #952 were treating symptoms.
Root cause:
DuckDbInitializer.ConnectionStringset nomemory_limit, so the buffer pool ran at DuckDB's default ceiling of 80% of system RAM (~12.8 GB). With archive parquet files accumulating, every UI query over archive views caches parquet pages — the buffer pool grows freely. That's the 2.7-2.9 GB the reporter is seeing.The wrinkle
We can't just cap the main connection at 1 GB statically. DuckDB v1.5.2 parquet
COPYhas a buffer-manager-bypass pre-reservation that needs 2-4 GB headroom (validated against DuckDB CLI v1.5.2 standalone — same OOM in both .NET binding and CLI; tracked upstream at duckdb#16482). So:memory_limit=1GBin the ConnectionString — caps the archive-page cache, addresses the actual complaint.SET memory_limit='4GB', run COPY, restore to1GB. Factored into aWithRaisedCopyMemoryLimithelper. Three call sites:ExportToParquetand the twoCOPYpaths inArchiveAllAndResetAsync.:memory:instances): keep 4 GB cap from Fix #933: raise compaction memory_limit to 4 GB #952.Separate Binder fix
CompactParquetFilesdetectedCompactionExcludeColumnsonce across the global union of files, then applied* EXCLUDE (col)to each pair.query_plan_textwas added toquery_store_statsin migration v13 (2026-02-23), so the reporter's mix of pre-v13 and post-v13 archives lets the global detector see the column, but a pair of two pre-v13 files hitsBinder Error: Column "query_plan_text" in EXCLUDE list not found in FROM clause. Fixed by detecting exclude-columns per merge-set in a newBuildSelectClausehelper.Validation
DuckDB CLI v1.5.2 against synthetic query_snapshots-shaped data:
*succeeds on 200 MB synthetic; OOMs on 1.5 GB. The 4 GB raise covers both.Binder fix reproduced and verified against the DuckDB CLI on a 1-file-with-column + 1-file-without pair.
Tradeoff (named, not hidden)
The resting 1 GB cap forces eviction of cached archive parquet pages. Long-range historical UI queries that re-scan many parquet files will do more disk I/O. Live/recent-data queries against the hot DB are unaffected — hot DB is small enough to fit in 1 GB easily.
Test plan
dotnet build Lite/PerformanceMonitorLite.csproj -c Releaseclean🤖 Generated with Claude Code