fix: RepartitionExec deadlock when its spill pool holds two open files by adriangb · Pull Request #24891 · apache/datafusion

adriangb · 2026-09-02T21:28:48Z

Which issue does this PR close?

Closes RepartitionExec deadlocks under a memory limit when its spill pool holds two open files #24883

Rationale for this change

Under a memory limit, a grouped aggregation on two or more partitions can stop and never complete. The query does not return and does not give an error. All worker threads wait. The issue has the full analysis.

In short: when RepartitionExec cannot reserve memory for a batch, it writes the batch to a spill pool and sends a marker. The reader then blocks on the pool until it gets a batch. When two input tasks spill at the same time, the pool has two open files. The reader reads only the oldest file. When that file has no unread batch, the reader waits for it. But the batch that the reader needs is in the newer file. When each channel has data, the gate closes and the writers wait too. Nothing can wake the reader.

What changes are included in this PR?

The PR has three commits, in test-first order.

test: reproduce the RepartitionExec spill pool deadlock with a SQL query. This is the query from the issue as a test in datafusion/core/tests/memory_limit/repartition_mem_limit.rs. It makes 12 attempts and stops each one after 20 seconds. On main, attempt 0 does not complete and the test fails with a message.
test: add a scenario fuzzer for the spill pool contract. This adds a test double that can hold a writer inside its first disk write, delay the reads of a file, or fail them. It adds a test that holds writer 1 while writer 2 pushes a batch, a test that a failed read gives an error to the reader, and spill_pool_scenario_fuzz, which runs random scenarios from a seed and checks the pool contract: after overlapping pushes the reader can read each pushed batch without more writer activity, it does not signal EOF while a writer is alive, it gives exactly the pushed batches (in order for one writer), and it releases the files. The first two tests are red on this commit. The extended CI job runs 1000 seeds.
fix: RepartitionExec deadlock when its spill pool holds two open files. SpillPoolReader now keeps all the files that it received, oldest first, and gives the first available batch. It skips a file that has no unread batch. It waits for the oldest file that has an unread batch when the read of that file is not complete, so one writer keeps FIFO order. The writer side does not change. The tests of the two commits before it become green.

The fuzzer finds each known fault of this pool that a test can reach:

Pool version	Unit tests of that version	Fuzzer
Before #23522 (has #23447)	16 of 16 pass	Seeds 0 and 1: stall with unread batches. Seed 4: stall after the drop of all writers (the #23447 signature).
`main` (has #24883)	pass	Seed 0: stall with 5 of 13 batches unread.
A draft of this fix that skips a file with a read in progress	1 failure	Seed 2: FIFO violation with one writer, `[1, 3, 4, 2, ...]`.
This fix	pass	500 seeds pass.

What is the testing strategy for this PR?

The reproducer makes 12 attempts, each with a 20-second limit. With the fix, 156 runs and 3000 instrumented attempts gave 0 stalls.
The memory limit must be small enough that RepartitionExec spills, and that window is narrow: at 4 MB the inner repartition spills in 10 of 10 runs, and at 6 MB and more it never spills. Thus the test also sums the spill_count metric of the RepartitionExec nodes and fails if no attempt spilled. A later change that stops the query from spilling makes the test say so, instead of passing while it covers nothing.
At 4 MB the greedy pool sometimes refuses an allocation of the final aggregate. That is a correct result of the limit and not a fault, so the test accepts a resources-exhausted error. One attempt in 3000 failed in this way.
The fuzzer runs 50 seeds by default, in approximately three seconds. DATAFUSION_SPILL_POOL_FUZZ_ITERATIONS sets the count, and DATAFUSION_SPILL_POOL_FUZZ_SEED replays one seed.
test_read_error_is_reported_to_the_reader covers the error paths of SpillPoolFile::poll_file, which had no test before.
The 33 memory-limit tests, the 111 spill and repartition unit tests, and the spill pool doctests pass. cargo fmt and cargo clippy --all-targets --all-features -- -D warnings pass on the two changed crates.

I also wrote this reproducer as a sqllogictest file and then removed it. sqllogictest has no time limit, so a return of the deadlock stops the file until the CI job reaches its time limit, instead of a quick failure with a message. It also cannot say "these rows, or this resource error", so the file failed 1 run in 400 (3200 queries) on the legitimate resources-exhausted error. The limit cannot be relaxed to avoid this, because at 6 MB and more nothing spills, and a smaller count of distinct values also stops the spill (with 97 distinct values 7 of 10 runs do not spill, and with 53 none do).

I also evaluated a second fix that keeps one open write file for each pool. It passes the same tests. It serializes writers that spill at the same time on the file lock, which #23522 avoided on purpose. It is not part of this PR.

Are there any user-facing changes?

No.

The branch-54 line has the same fault, because #23654 backported the same file model. A backport of this fix is necessary.

🤖 Generated with Claude Code

https://claude.ai/code/session_01RauD5PqPCp4y92RUZUD6C9

adriangb

Small notes

Copilot

🔵 Needs a closer look

Spill coordination is concurrency-sensitive, and the seed replay overflow remains unresolved.

Pull request overview

Fixes a RepartitionExec deadlock caused by concurrent spill files under memory pressure.

Changes:

Reads available batches across active spill files.
Adds SQL, regression, and fuzz coverage.
Expands extended-CI fuzzing to 1,000 scenarios.

File summaries

File	Review
`datafusion/sqllogictest/test_files/repartition_memory_spill.slt`	Adds SQL-level spill and result validation.
`datafusion/physical-plan/src/spill/spill_pool.rs`	Fixes multi-file coordination and adds concurrency tests. Moderate (1 vote): seed-range addition can overflow for accepted `u64` replay values.
`datafusion/core/tests/memory_limit/repartition_mem_limit.rs`	Adds a timeout-based regression test.
`.github/workflows/extended.yml`	Runs 1,000 spill-pool fuzz scenarios.

Review details

Files reviewed: 4/4 changed files
Comments generated: 1
Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

codecov-commenter · 2026-09-02T21:56:30Z

Codecov Report

❌ Patch coverage is 88.59416% with 43 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.60%. Comparing base (da89c7c) to head (b7a9555).
⚠️ Report is 39 commits behind head on main.

Files with missing lines	Patch %	Lines
datafusion/physical-plan/src/spill/spill_pool.rs	88.59%	25 Missing and 18 partials ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##             main   #24891     +/-   ##
=========================================
  Coverage   81.60%   81.60%             
=========================================
  Files        1123     1123             
  Lines      408898   411514   +2616     
  Branches   408898   411514   +2616     
=========================================
+ Hits       333670   335806   +2136     
- Misses      55625    55928    +303     
- Partials    19603    19780    +177

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Three changes from the review of apache#24891. `spill_pool_scenario_fuzz` added the iteration count to the first seed. That addition can overflow when a person replays a seed near the maximum of a `u64`. It now uses `saturating_add`. The SLT file asserted the spill with `<slt:ignore>` markers around the metrics of `RepartitionExec`. The matcher splits the expected text on the marker and then finds each fragment in order in the full snapshot, not in one line. The assertion thus did not say that the spilled bytes of that operator are in KB, only that the text `KB,` comes after the text `spilled_bytes=` somewhere in the plan. It also needs the spilled bytes to stay in the KB range; 25 samples gave 34.9 KB to 67.8 KB, and a change to the batch sizes could move the value to B or MB and fail the test for no good reason. The EXPLAIN ANALYZE block is gone, and the Rust test now sums the `spill_count` metric of the `RepartitionExec` nodes and fails if no attempt spilled. That is the property the block tried to assert. The read hook of the test double can now fail a read, and the new test `test_read_error_is_reported_to_the_reader` makes sure that a spill file that cannot be read gives an error to the reader instead of stopping it. `RepartitionExec` waits on this stream after each spilled marker, so an error that gets lost here stops the query in the same way as the deadlock that this module guards against. The error arms of `SpillPoolFile::poll_file` had no test before this one. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01RauD5PqPCp4y92RUZUD6C9

Copilot

🔵 Needs a closer look

A moderate fuzz-seed iteration issue remains unresolved, alongside a test-coverage nit.

Review details

Suppressed comments (1)

datafusion/physical-plan/src/spill/spill_pool.rs:1750

The injected failure is returned by read_stream() itself, so this test only exercises stream construction at poll_file line 638. It never covers the refactored path where an existing reader stream yields Some(Err(_)) at line 660. Return the injected error as the first stream item instead so the test validates that polling path.

            let delay = (self.read_hook)()?;
            let mut inner = Some(self.inner.read_stream()?);
            Ok(Box::pin(
                futures::stream::once(tokio::time::sleep(delay))
                    .flat_map(move |_| inner.take().expect("polled once")),
            ))

Files reviewed: 3/3 changed files
Comments generated: 1
Review effort level: Balanced

Add the query from issue apache#24883 as a test. It is a grouped COUNT(DISTINCT) on a Utf8View column, with a 4 MB memory limit, two partitions and 64-row batches. Those values send `RepartitionExec` into its spill path. On the current code, attempt 0 does not complete. The test stops each attempt after 20 seconds and fails with a message. It makes 12 attempts, because one attempt that completes is not sufficient for a pass. The memory limit must be small enough that `RepartitionExec` spills, and that window is narrow: at 4 MB the inner repartition spills in 10 of 10 runs, and at 6 MB and more it never spills. Thus the test also sums the `spill_count` metric of the `RepartitionExec` nodes and fails if no attempt spilled. If a later change stops the query from spilling, the test says so instead of passing while it covers nothing. At 4 MB the greedy memory pool sometimes refuses an allocation of the final aggregate. That is a correct result of the limit and not a fault, so the test accepts a resources-exhausted error. In 3000 attempts, one attempt failed in this way. The module is not compiled with `force_hash_collisions`. That feature makes every hash 0, so the hash `RepartitionExec` sends all rows to one output partition. The gate of the distributor channels closes only when each channel holds data, thus the deadlock cannot occur and the test cannot show it. The feature also makes the group-by hash table degenerate, which made one attempt of this query approximately 36 times slower and thus longer than the limit above. A sqllogictest file is not suitable for this test. sqllogictest has no time limit, so a return of the deadlock stops the file until the CI job reaches its time limit, instead of a quick failure with a message. It also cannot say "these rows, or this resource error", so it would fail approximately 1 CI run in 400. Ref: apache#24883 Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01RauD5PqPCp4y92RUZUD6C9

The spill pool had three faults since its unbuffered reads: apache#20683, apache#23447 and apache#24883. In each fault, two writers were in `push_batch` at the same time. No unit test could cause that overlap. This commit adds a test double and three tests. `IoHookFactory` is a `TempFileFactory` that runs a hook before each disk write and consults a second hook before each read. A writer writes while it holds the lock of its file. Thus a hook that blocks can hold a writer at the point where it has a file checked out, a read delay makes the bytes of an older file arrive after those of a newer file, and a read hook can also fail a read. `test_reader_does_not_wait_on_drained_file_while_another_has_data` holds writer 1 inside its first write while writer 2 pushes a batch. The reader must then give both batches while both writers are alive. `test_read_errors_are_reported_to_the_reader` fails the read of a spill file in the two possible ways: the file does not open, and the file opens and then gives an error as the first item of its stream. Those are the two error paths of `SpillPoolFile::poll_file`, and the reader must give the error in both cases. `RepartitionExec` waits on this stream after each spilled marker, so an error that gets lost here stops the query in the same way as a deadlock. `spill_pool_scenario_fuzz` runs random scenarios from a seed: one to four writers, file rotation after each batch, after some batches or never, and one to six phases. In each phase a random set of writers pushes one to three batches each. A pauser holds each writer of the phase inside its first write, so the pushes overlap. Then the reader reads a random number of the available batches, which can leave a backlog for a later phase. Some phases drop a writer. Each scenario checks the contract that `RepartitionExec` depends on: - When no push is in progress, the reader can read each pushed batch without more writer activity. - The reader does not signal EOF while a writer is alive. - After the last writer is dropped, the reader gives the backlog and then EOF. - The reader gives exactly the pushed batches. With one writer, it gives them in push order. - Disk usage is zero after the reader is dropped. The first two tests are red on this commit. The fuzzer also fails on these versions of the pool: - The code before apache#23522. Seed 4 shows the apache#23447 stall after the drop of all writers, and seeds 0 and 1 show the apache#24883 stall. All 16 pool tests of that time pass on that code. - A draft of the fix in the next commit that skips a file with a read in progress. Seed 2 shows a FIFO violation with one writer. The default of 50 seeds takes approximately three seconds. The extended CI job runs 1000 seeds. `DATAFUSION_SPILL_POOL_FUZZ_SEED` replays one seed. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01RauD5PqPCp4y92RUZUD6C9

In non-preserve-order mode, all input tasks of a `RepartitionExec` share one spill pool for each output partition. When two input tasks spilled at the same time, the pool had two open spill files. The `SpillPoolReader` read only the oldest file. When that file had no unread batch and was not finished, the reader waited for it. But the batch that the reader needed was in the newer file. When each distributor channel had data, the gate closed. Then both input tasks stopped in `send`, no sink was dropped, and nothing could wake the reader. The reader now keeps all the files that it received, oldest first. It gives the first batch that is available in one of them. It skips a file that has no unread batch. It waits for the oldest file that has an unread batch when the read of that file is not complete. Thus one writer gets its batches in FIFO order. This commit makes the tests of the two commits before it green. The fuzzer passes 500 seeds. Ref: apache#24883 Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01RauD5PqPCp4y92RUZUD6C9

adriangb · 2026-09-03T15:11:54Z

@2010YOUY01 @kosiew would one of you mind reviewing this change? Thanks!

kosiew

@adriangb
Thanks for working on this. The fix looks good to me, and I like the additional direct regression, error, fuzz, and end-to-end coverage. I have one non-blocking suggestion to make the direct concurrency regression test a little more deterministic.

kosiew · 2026-09-04T09:04:20Z

+        });
+        // Let writer 2 either complete (into a second file) or block on writer
+        // 1's file lock, depending on the pool implementation.
+        std::thread::sleep(Duration::from_millis(200));


Could we replace the fixed 200 ms sleep with a synchronization point that confirms writer 2 has acquired or created its file before releasing writer 1? On a heavily loaded runner, writer 2 might not get scheduled until after writer 1 is released. In that case it could reuse writer 1's returned file, leaving the test with only one file. The test could then pass on the old implementation without actually exercising the two-open-file case. This is just test hardening, since the fuzzer gives us additional coverage as well.

Addressed in b7a9555

…test The two-open-file regression test slept 200 ms to let writer 2 push while writer 1 was held inside its first disk write. On a loaded runner writer 2 could be scheduled only after writer 1 was released, and would then reuse writer 1's returned file. The run would pass with a single open file, which says nothing about the case the test is for. The write hook now reports every write other than the held one. A write gets there only once its writer holds a file of its own, so the test waits for that report before it releases writer 1, and then asserts that two files were written. Ref: apache#24891 (comment) Co-Authored-By: Claude Opus 5 <[email protected]>

adriangb · 2026-09-04T15:38:01Z

Thanks for the review @kosiew !

A RepartitionExec that cannot reserve memory spills its input tasks' batches to one pool per output partition. Its reader drains only the oldest file of the pool: when two input tasks spill at once the pool holds two, the reader waits on a drained one while its next batch is in the other, the distributor gate closes, and every worker parks for good (apache/datafusion#24883). A TPC-H Q8 at scale factor 10 stopped so on all three segments under a 32 MB budget, its workers idle, until cancelled. The fix (apache/datafusion#24891) is on main only: 55.2.0 ships the reader unchanged. rust/vendor/datafusion-physical-plan is the published 55.2.0 crate with that change to spill_pool.rs, put in place by [patch.crates-io] until a release carries it (rust/vendor/README.md). tests/repartition_spill.rs, the fix's test of the pool through the public interface, times out on the published crate and passes on this one. Regression 19/19; SHJ stress 33 queries, 0 diffs; TPC-H SF1 and SF10 44/44 matching PostgreSQL, times unchanged (SF10 52.0 s).

…(Part of #1985) Select vendor/datafusion-physical-plan through [patch.crates-io] and keep vendor/* out of the workspace so fmt, clippy and the workspace test run never touch code we do not own. GRAPHFORGE.md records the origin, the one change to be applied (apache/datafusion#24891, fixing #24883) and when to delete the copy. The vendored tree trips scripts/license_check.py: the Apache Software Foundation header on 121 upstream files says "contributor license agreements", which the forbidden-claim scan rejects. The scan now skips third-party files under vendor/ and still scans the GRAPHFORGE.md we write there and every first-party file. Co-Authored-By: Claude Sonnet 5.5 <[email protected]>

) into the vendored crate (Part of #1985) Port of apache/datafusion#24891 (upstream 09a2aff7) to the vendored datafusion-physical-plan 54.1.0 spill pool. RepartitionExec deadlocks under a memory limit when its spill pool holds two open files (apache/datafusion#24883): the reader waited on a drained, still-open file while the batch it was owed sat in a newer file, and nothing could wake it. The reader now owns the list of files it has picked up (new files are published through a `new_files` queue), polls them oldest first, skips a file that is only caught up with its writer, and waits only on a file whose unread batches are still being read from disk. Only spill_pool.rs changes; names and docs are adapted to the 54.1.0 API (SpillFile, active_writer_count, RefCountedTempFile, one cloneable SpillPoolWriter and one channel function). Tests added to the same file: the two-open-files drain case adapted from upstream (writer 1 is simulated mid-push by checking its file out of open_write_files), a wake-path case, and a poll-time read-error case. Upstream's scenario fuzz test needs 55-only I/O hooks and is not ported. Co-Authored-By: Claude Sonnet 5.5 <[email protected]>

…uest A sort that spills merges its runs, in partitions that end their input early, with as many buffers as the pool grants, and a partition still sorting then finds no memory left (apache/datafusion#25804, finding 9): TPC-DS Q67 on GPORCA failed so 10 times in 10, and the window queries of WIN1 (the three largest per group by rank(), a running sum) did not finish. apache/datafusion#26264 fixes it. rust/Cargo.toml now takes DataFusion from a commit of that pull request's branch (MisterRaindrop/datafusion 5c59c075f, main with the fix) until a release has it. Main also has the spill pool fix (apache/datafusion#24891) that rust/vendor carried for 55.2.0: the vendored crate and the [patch.crates-io] go. Main's hash join reserves more for its build side than 55.2.0's did: the copy it concatenates the batches into, while it still holds them (apache/datafusion#25496). SpillingHashJoin collected a build side under its size and 40 bytes a row, which the join had used within; past it, the join reserved as memory that cannot spill, which a pool the partitions' build sides had filled did not grant: TPC-DS Q4 failed under both planners and two SHJ joins under work_mem 2 MB. A build side read to its end now also reserves what the join will take beyond: its table as DataFusion estimates it (on one integer key, the most an array over the keys' range may take, which the join builds where the keys are dense: a LEFT JOIN of SHJ failed for it one run in a dozen) and the copy. One that does not get it spills. Two tests, each failing without its part. An aggregate now reserves for its accumulators when it starts (apache/datafusion#24692), which a pool full to its limit does not grant: the test of a join that fits in a full pool counts its rows itself. Rust tests 10 runs of 10; regression 22/22; TPC-DS SF1 205/206 matching PostgreSQL (Q71's ties), 83.9 s against 133.8 s; Q67 on GPORCA 10/10; TPC-H SF1 and SF10 44/44, 9.4 s and 55.3 s against 31.2 s and 205.0 s; SHJ 15 runs and work_mem 1 MB without a diff; the window queries finish with PostgreSQL's rows, top 3 2.1 s against 9.0 s, running sum 3.0 s against 6.4 s.

Under a memory limit, lakesoul-console hung on TPC-H q17 over tpch_sf1_pkmonth (2 CPU, LAKESOUL_MEMORY_LIMIT=1Gi, LAKESOUL_CACHE=1) in about one run in four: every task parked, no error. EXPLAIN ANALYZE showed the hash RepartitionExec spilling. This is apache/datafusion#24883: two input tasks spilling at once leave the shared spill pool with two open files, the reader waits on the drained older file while the batch it needs is in the newer one, and the writers park on the closed distributor gate. The fix (apache/datafusion#24891, merge commit 09a2aff7) is only in DataFusion 56. Vendor datafusion-physical-plan 55.2.0 under rust/third_party with that commit's src/spill/spill_pool.rs change and use it via [patch.crates-io]. With the patch the same configuration still spills in RepartitionExec and completed 20 of 20 runs; the crate's 20 spill_pool tests, including the upstream scenario fuzzer, pass. Co-Authored-By: Claude Opus 5.5 <[email protected]> Change-Id: I6f24b7436d4bcd3ee74bfd3cccaff5ae6a6a6964 Co-Authored-By: Codex <[email protected]> Signed-off-by: mag1cian <[email protected]>

…rage (#1249) * [PG]: add Kubernetes benchmark harness for postgres-lakesoul Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: mag1cian <[email protected]> Change-Id: I60005b72765058a41c7722793149e3786a6a6964 * [PG]: bench add pkmonth layout Signed-off-by: mag1cian <[email protected]> Change-Id: I173773455d5641f7bd29861de4ae0b916a6a6964 * [PG]: export query outcome and memory failure metrics at zero on startup Pre-register every outcome label of lakesoul_query_requests_total, lakesoul_query_rows_total and lakesoul_query_duration_seconds at 0 when the coordinator starts, so increase()/rate() see the first burst of an outcome instead of the series appearing at its first nonzero sample. lakesoul_memory_reservation_failures_total was already registered at pool creation; use increment(0) instead of absolute(0) so a second pool in the same process cannot reset it. Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: mag1cian <[email protected]> Change-Id: Icb291d45a0dcadcda90f6f6f7f3ed92c6a6a6964 * [PG]: report memory failures from the series bench collects report.py looked up increase(lakesoul_memory_reservation_failures_total[15s]) while bench.py collects the [1m] window (a [15s] window holds at most one sample at a 15 s scrape interval), so the memory table always showed an empty failures column. report.py now imports the memory queries from bench.py instead of retyping them, and the column says it is the max per 1m window. Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: mag1cian <[email protected]> Change-Id: I54e4ceb6686155de1582d6fc302de2fa6a6a6964 * [Rust]: export worker discovery refresh failures, snapshot age and eligible workers The r07 discovery outage was diagnosable only from coordinator logs. The Kubernetes resolver now exports lakesoul_worker_discovery_refresh_failures_total{kind="hard"|"transient"}, lakesoul_worker_discovery_snapshot_age_seconds (since the last successful refresh, updated after every attempt, +Inf before the first) and lakesoul_worker_discovery_eligible_workers (also set by static discovery). The series are registered when the resolver starts, so the first outage shows in increase(). The benchmark collects them. Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: mag1cian <[email protected]> Change-Id: I6a99af0d7882d6c2bc2ccd09501b5bb76a6a6964 * [PG]: scrape container memory, restarts and throttling in the k8s observability overlay The processes cannot report their own OOM kills, restarts or CPU throttling, and the overlay could not show them. lakesoul-prometheus now scrapes every kubelet's cAdvisor (ClusterRole with nodes list/watch and nodes/metrics, not nodes/proxy), keeping only the LakeSoul containers' memory and CFS series, and a namespaced kube-state-metrics deployed by the overlay (--namespaces=lakesoul --resources=pods) for restart counts and last termination reasons. The LakeSoul Overview dashboard gains panels for container RSS / working set / limit vs. lakesoul_memory_pool_reserved_bytes and the pool limit, restarts and OOM kills (last_terminated_reason, since cAdvisor's OOM event series vanishes with the killed container), and CPU throttling. Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: mag1cian <[email protected]> Change-Id: Ia3628d12ea9c8bc897592e3951c048de6a6a6964 * [PG]: run lakesoul-console TPC-H benchmarks as Guaranteed-QoS k8s Jobs lsbench gains a `console` scenario that runs the TPC-H queries through one lakesoul-console process (single node or distributed over --worker URLs) and turns its per-statement Elapsed output into the standard sample records, with row-count answer checks. bench.py console runs it in a Job whose containers have requests equal to limits, a native sampler sidecar, worker URLs from the lakesoul-worker EndpointSlices, and samples read from the container log so an OOM-killed console still reports finished queries. Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: mag1cian <[email protected]> Change-Id: Ie8978dc91b36dc8845cbb1eb3b9c494f6a6a6964 * [Rust]: add JSON output to lakesoul-console and value-check console benchmarks lakesoul-console --format json prints, per statement, a column line with names and Arrow types, then one JSON array per row with the values in the table format and NULL as null. lsbench console uses it to compare results with answers.compare, as the tpch scenario does, instead of row counts. Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: mag1cian <[email protected]> Change-Id: Ib9fd809244ee3e44e6c07663239376b26a6a6964 * [PG]: pass AWS_S3_VIRTUAL_HOST_STYLE from lakesoul-secrets to k8s pods Huawei OBS endpoints such as cn-north-4 reject path-style requests with VirtualHostDomainRequired, and LakeSoul defaults to path style. The coordinator, worker, datagen and console manifests now read AWS_S3_VIRTUAL_HOST_STYLE from the optional secret key aws-s3-virtual-host-style; without the key, behavior is unchanged. Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: mag1cian <[email protected]> Change-Id: I4497426534454ef038903bd286dfa1046a6a6964 * [PG]: document the regenerated TPC-H benchmark data set List the six tpch_sf{1,10}{,_pkmonth,_nopk} schemas with the datagen flags used, and record that the SF10 pkmonth and nopk loads needed lineitem rerun alone with a higher memory limit. Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: mag1cian <[email protected]> Change-Id: I78a683aede606cd31bcbb0b2bece9d706a6a6964 * [PG]: chunk nopk TPC-H loads by whole tpchgen parts The nopk sink writes one file per tpchgen part. Modulo chunks put a slice of every part in every statement, so CHUNKS=4 wrote 4x the files at 1/4 the size and changed the scan layout. For LAYOUT=nopk, chunk k is now the key range of parts [k*PARTS/N, (k+1)*PARTS/N), computed from tpchgen's part boundaries and order key mapping, so each part is still one file. PARTS must be a multiple of CHUNKS; nation/region stay single statements. Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: mag1cian <[email protected]> Change-Id: I122cbb9acd1c7c70f3e40ed427a6f7846a6a6964 * [Rust]: honor memory limit and LAKESOUL_CACHE in lakesoul-console lakesoul-console built its session factory with LakeSoulSessionFactory::new, whose pool is ExecutionMemoryArgs::default(): LAKESOUL_MEMORY_LIMIT was ignored, execution was unbounded and never spilled. The console now takes ExecutionMemoryArgs (--memory-limit, --memory-pool, --vortex-decode-budget and their env) and installs the pool and Vortex decode budget like the coordinator. Sessions register the warehouse S3 store with register_s3_object_store, which skipped the disk cache; only register_object_store honored LAKESOUL_CACHE, and it returns early once a store exists. The check now lives in register_s3_object_store, so coordinator, worker and console sessions read the warehouse through the shared cache when it is set. Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: mag1cian <[email protected]> Change-Id: I463757c12ebd13d2307c0f708ad0f6866a6a6964 * [Rust]: patch DataFusion RepartitionExec spill-pool deadlock Under a memory limit, lakesoul-console hung on TPC-H q17 over tpch_sf1_pkmonth (2 CPU, LAKESOUL_MEMORY_LIMIT=1Gi, LAKESOUL_CACHE=1) in about one run in four: every task parked, no error. EXPLAIN ANALYZE showed the hash RepartitionExec spilling. This is apache/datafusion#24883: two input tasks spilling at once leave the shared spill pool with two open files, the reader waits on the drained older file while the batch it needs is in the newer one, and the writers park on the closed distributor gate. The fix (apache/datafusion#24891, merge commit 09a2aff7) is only in DataFusion 56. Vendor datafusion-physical-plan 55.2.0 under rust/third_party with that commit's src/spill/spill_pool.rs change and use it via [patch.crates-io]. With the patch the same configuration still spills in RepartitionExec and completed 20 of 20 runs; the crate's 20 spill_pool tests, including the upstream scenario fuzzer, pass. Co-Authored-By: Claude Opus 5.5 <[email protected]> Change-Id: I6f24b7436d4bcd3ee74bfd3cccaff5ae6a6a6964 Co-Authored-By: Codex <[email protected]> Signed-off-by: mag1cian <[email protected]> * [Rust]: let DATAFUSION_EXECUTION_BATCH_SIZE set the session batch size create_lakesoul_session_config read the DataFusion environment with SessionConfig::from_env() and then forced .with_batch_size(8192), so DATAFUSION_EXECUTION_BATCH_SIZE was silently ignored and the batch size could only be changed per session with SET. 8192 is DataFusion's default, so dropping the override changes nothing when the variable is unset. The batch size governs memory held outside LAKESOUL_MEMORY_LIMIT: at a 1Gi budget, TPC-H SF10 q09 peaked at 2.2 GiB RSS with 8192 rows per batch and 0.76 GiB with 2048. Document the variable, and that workers do not read it (datafusion-distributed builds worker tasks from DataFusion's defaults plus the coordinator's distributed.shuffle_batch_size). Co-Authored-By: Claude Opus 5.5 <[email protected]> Change-Id: I4c829aed5a8c706bf7cadbaab4c4fec06a6a6964 Co-Authored-By: Codex <[email protected]> Signed-off-by: mag1cian <[email protected]> * [IO]: pack MOR range partitions into one bounded merge per scan task A merge-on-read scan built one MergeParquetExec per range partition. Its parent pulls every input partition at once, so a table with 84 range partitions opened 84 merges together, each holding a non-spillable LakeSoulMerge reservation, whatever target_partitions was: pkmonth q01 exhausted a 1 GiB pool at target_partitions = 2. The scan now packs whole range partitions into one merge per task the scan stage may get (scan_task_budget: the distributed per-stage cap, or the listed workers; 1 for a local plan or the local fallback), each with at most target_partitions lanes. A lane runs its bucket work units one at a time, so a local scan, or one distributed task, holds at most target_partitions merges of the scan open. Work units keep their own files and range constants, so MOR version order, partial updates and range defaults are unchanged, and a range never spans two tasks. Co-Authored-By: Claude Opus 5.5 <[email protected]> Change-Id: I408225af368ecbe11b680ae84615d2236a6a6964 Co-Authored-By: Codex <[email protected]> Signed-off-by: mag1cian <[email protected]> * [IO]: do not sample open merges after the first batch in the early-drop test CoalescePartitionsExec producers keep polling after sending a batch, so a merge may already be closed when the consumer receives the first batch. Assert that a merge executed instead; the lakesoul-io lane test pins the release of a merge that is open mid-unit. Co-Authored-By: Claude Opus 5.5 <[email protected]> Change-Id: I8b91213ab7d12b1e0724d99273afed5b6a6a6964 Co-Authored-By: Codex <[email protected]> Signed-off-by: mag1cian <[email protected]> * [Rust]: test where the session batch size comes from DataFusion default 8192 without an override, DATAFUSION_EXECUTION_BATCH_SIZE when set, a session SET over both, and LakeSoul-fixed options such as prefer_hash_join over the environment. A separate test binary with one test, so changing the process environment races nothing else. Co-Authored-By: Claude Opus 5.5 <[email protected]> Change-Id: Ibd0fa4abe6e75932f37620b250a70b456a6a6964 Co-Authored-By: Codex <[email protected]> Signed-off-by: mag1cian <[email protected]> * [PG]: accept any order of rows with equal sort keys in lsbench answers TPC-H q11 orders by `value` alone, and its SF10 answer has two pairs of parts with equal values. Every q11 "mismatch" of the SF10 console runs swaps one of these pairs, and the ordered check stopped at the first difference, so the rest of the result was never checked. When every ORDER BY key is a bare output column, rows with equal keys now match as a multiset at their positions; other keys keep exact row order. Co-Authored-By: Claude Opus 5.5 <[email protected]> Change-Id: Iac26b16e21c686e17b450273eb5c00616a6a6964 Co-Authored-By: Codex <[email protected]> Signed-off-by: mag1cian <[email protected]> * [Rust]: make jemalloc the console binaries' allocator; drop hard-coded profiling lakesoul-io no longer installs a global allocator. Its `jemalloc` feature installed jemalloc with heap profiling forced on via `malloc_conf` and a developer-specific `prof_prefix` path. The feature is now stats-only (`resident_memory()` for LoggedMemoryPool); the unused flamegraph and stats-print helpers and the jemalloc_pprof dependency are gone. postgres-lakesoul and lakesoul-console now own `#[global_allocator]`: jemalloc 5.3 with built-in defaults (no profiling compiled in, no decay tuning) behind a default `jemalloc` feature; `--no-default-features` gives the system allocator. `unprefixed_malloc_on_supported_platforms` routes C `malloc` through jemalloc too. Each binary logs the active allocator at startup (version, config.prof, decay options, stats.allocated). lakesoul-worker, lakesoul-io-c (JNI), lakesoul-metadata-c, Python and the libraries do not link jemalloc and keep the system allocator. Co-Authored-By: Claude Opus 5.5 <[email protected]> Change-Id: I18781042bb92889610b11c1cd17290c16a6a6964 Co-Authored-By: Codex <[email protected]> Signed-off-by: mag1cian <[email protected]> * [PG]: validate jemalloc console allocator on SF10 pk (7100c918) Interleaved q09 A/B against the glibc control image e04e8205703e on the THP=always node (2 CPU, 3 GiB, 1 GiB fair pool, batch 8192), plus the full pk suite against the 20261024 glibc baseline. jemalloc lowers peak anon RSS from 2.63 to 1.81 GiB on q09 and from 2.87 to 1.84 GiB over the suite, with identical results, spill volume and total CPU, at ~16x (q09) to ~41x (suite) more minor faults. Document the allocator, its startup log line and rollback in the k8s and benchmark READMEs. Co-Authored-By: Claude Opus 5.5 <[email protected]> Change-Id: I8b2b289b138b33281aa7972c697190a96a6a6964 Co-Authored-By: ChatGPT <[email protected]> Co-Authored-By: Codex <[email protected]> Signed-off-by: mag1cian <[email protected]> * [Meta]: preserve secondary workspace after history cleanup Retain the existing empty workspace state on the rewritten benchmark history without changing its files. Co-Authored-By: ChatGPT <[email protected]> Change-Id: I160644a5aa0de15e35107080b1c887566a6a6964 Co-Authored-By: Codex <[email protected]> Signed-off-by: mag1cian <[email protected]> * [PG]: compatiable test stage 1 Change-Id: I1a2a0de511ae2a12d29b96f5eb856ec06a6a6964 Co-Authored-By: Codex <[email protected]> Signed-off-by: mag1cian <[email protected]> * [PG]: compatibility stage 2 Change-Id: I9a3d72a98723593953ea84e0f71c31d66a6a6964 Co-Authored-By: Codex <[email protected]> Signed-off-by: mag1cian <[email protected]> * [PG]: compatibility stage 3 and 4 Stage 3 contracts with physical proofs: - storage: append duplicates, merge superseded values, partial columns, partition-local merge, CDC tombstone removal - catalog: namespace -> database mapping, public schema, no cross-database read via search_path - vector: codec, exact fallback, ANN route and recall, probes isolation Stage 4 strict TPC-H gate: - lsbench strict driver with SF 0.1/1/10 answers and negative exits - tpch-compat workflow; PK single coordinator, nopk/pkmonth dual worker Session and protocol fixes: - connection-local SET/RESET/DISCARD TIME ZONE with startup defaults and plan invalidation on settings generation - SHOW Describe reports the column it executes: hooks claim SHOW at Parse, responses carry text (OID 25) like PostgreSQL Change-Id: Ia029cbf7b6799a40c97170ba3b90152e6a6a6964 Co-Authored-By: DeepSeek V4 Flash <[email protected]> Co-Authored-By: Codex <[email protected]> Signed-off-by: mag1cian <[email protected]> * [PG]: borrow pg-catalog from the fork instead of vendoring it The RemoveSubqueryFromProjection removal now lives on the datafusion-postgres fork (mag1c1an1/datafusion-postgres, lakesoul-shared-static-tables, commit 03441f8) instead of in a local copy of the crate. datafusion-pg-catalog is a path dependency inside that fork workspace, so pinning a newer rev is enough. - bump the datafusion-postgres rev to 03441f8 - drop the [patch."https://github.com/mag1c1an1/datafusion-postgres.git"] entry - drop the workspace exclude for rust/third_party/datafusion-pg-catalog - delete rust/third_party/datafusion-pg-catalog (96 files, -9k lines) Verified after the switch: --bins 121 passed / 1 ignored and the six targets 16 passed / 1 ignored in both merge configurations on PostgreSQL 17.10; clippy --bin postgres-lakesoul --tests --no-deps -- -D warnings clean; metadata, reference schemas and connections back to zero. Co-Authored-By: DeepSeek V4 Flash <[email protected]> Change-Id: I0a2387bb227ba54c1feb3b75c529132c6a6a6964 Co-Authored-By: Codex <[email protected]> Signed-off-by: mag1cian <[email protected]> * [PG]: add CREATE TABLE and DROP TABLE to the front end Add src/ddl.rs: a statement hook, installed per connection ahead of the read-only guard, that answers PostgreSQL `CREATE TABLE` and `DROP TABLE [IF EXISTS]` instead of DataFusion's own DDL planning. A table is built with `LakeSoulTableProvider::new_from_create_external_table` and published through `LakeSoulNamespace::register_table`, so its metadata, primary-key partition layout and `WITH (hashbucketnum, file_format)` properties are exactly what the LakeSoul management SQL produces, while the location stays `{warehouse}/{database}/{table}` and cannot be chosen by the client. Supported: the PG column subset smallint/int/bigint/real/double/boolean/ varchar/char/text/date/timestamp/timestamptz/decimal(p,s)/bytea, one primary key (column or table constraint), `IF NOT EXISTS`, `DROP TABLE IF EXISTS` with several names, and extended-protocol Parse/Execute. Refused with an explicit SQLSTATE: CREATE TABLE AS (needs the INSERT slice), TEMPORARY/EXTERNAL/OR REPLACE, LIKE/CLONE, STORED AS/LOCATION, UNIQUE/CHECK/FOREIGN KEY, column defaults, other types and options (0A000), out-of-range hashbucketnum (22023), unknown schema (3F000), and any DDL inside a transaction block, because LakeSoul metadata commits immediately and would silently ignore ROLLBACK. Duplicate tables are 42P07; dropping an absent table is 42P01. tests/ddl_contract.rs proves each step through the wire protocol against the real child process and the metadata database: table_info namespace, path and properties, pg_class, column OIDs, duplicate/IF NOT EXISTS identity, cascaded drop counts, the refusal matrix (nothing published, connection usable), extended protocol, and the transaction boundary. The harness cleanup now enumerates table_info per namespace before deleting it, so tables a suite created through the server are reclaimed too. Verified on PostgreSQL 17.10 in both merge configurations: seven targets 18 passed/1 ignored each, --bins 122 passed/1 ignored, clippy -D warnings and rustfmt clean, metadata residue zero. Co-Authored-By: DeepSeek V4 Flash <[email protected]> Change-Id: Ic4040e641174f8e41d299ae1820b93dc6a6a6964 Co-Authored-By: Codex <[email protected]> Signed-off-by: mag1cian <[email protected]> * [PG]: tighten the DDL contract after review Address the reviewer findings on the CREATE/DROP TABLE slice. Identifiers are folded the way PostgreSQL folds them (unquoted to lower case), and a name that folding would change, or that would alter the data path or the partition description (`/`, `\`, `?`, `#`, `%`, `,`, `;`, control characters, `.`/`..`), is refused instead of producing a location outside `{warehouse}/{database}` or a `partitions` string that decodes to different keys. This also makes the metadata identity consistent: the provider folds column and table names itself, so a quoted mixed-case name used to publish `id` as the field and `;ID` as the key. CREATE TABLE now refuses every parsed clause LakeSoul cannot honour (DEFERRABLE and other constraint characteristics, USING/INCLUDE on the key, INHERITS, PARTITION BY/PARTITION OF, ON COMMIT, ON CLUSTER, Hive distribution/row format, WITHOUT ROWID, STRICT, VERSION, COMMENT, ...) instead of silently ignoring it, rejects duplicate columns (42701), repeated key columns (42P16), a key naming a missing column (42703), conflicting NULL/NOT NULL declarations (42601), FLOAT(0) and non-default timestamp precision, and reports `hashbucketnum = -1` as 22023. DROP TABLE resolves and validates every name before the first deletion, so a statement that PostgreSQL would reject leaves all named tables and their metadata in place. A simple-query message that mixes DDL with other statements is refused with 0A000 before its first statement runs: PostgreSQL would execute the batch in an implicit transaction and roll the CREATE back when a later statement fails, while LakeSoul table metadata is committed immediately. Tests: the drop cascade is now counted by the table_id captured before the DROP, pg_class queries are parameterized, and the new cases cover the identifier rules, all-or-nothing multi-name DROP, the batch refusal, IF NOT EXISTS keeping the existing schema, extended-protocol refusals, and the tightened error matrix. Verified on PostgreSQL 17.10 in both merge configurations: seven targets 18 passed/1 ignored each, --bins 122 passed/1 ignored, clippy -D warnings and rustfmt clean, metadata residue zero. Co-Authored-By: DeepSeek V4 Flash <[email protected]> Change-Id: I29b3887605aea551c3ef411943edf7a36a6a6964 Co-Authored-By: Codex <[email protected]> Signed-off-by: mag1cian <[email protected]> * [PG]: support INSERT with merge-on-read conflict semantics Add the PostgreSQL front end's INSERT hook and the write-side changes it relies on: - accept INSERT in the read-only guard; the hook resolves the target namespace from the parsed DML target, maps ON CONFLICT onto LakeSoul write modes, refuses unsupported clauses with PostgreSQL SQLSTATEs and reports INSERT 0 n. - add a per-statement conflict mode to the session and typed WriteViolation/NotNullViolation errors mapped to 23505/23502. - publish every range partition of one statement in a single metadata transaction instead of one commit per partition. - do not publish when the result stream was cancelled before the commit started; remove the attempted files instead. Follow-ups from review: - delete a statement's attempted files when metadata publication fails, but only after checking by commit id that no partition became visible: a dropped connection during COMMIT leaves the outcome unknown. - map InsertConflict/CommitConflict to SQLSTATE 40001 (retryable) instead of XX000; the loser of a write race cannot be classified as 23505 until it retries. - publish multi-partition commits in partition_desc order so concurrent statements cannot take opposite partition orders. - document that index maintenance after a successful commit still reports an error while the rows are committed and visible. - add a multi-partition INSERT case over a non-PK range table (three partitions, one statement). - rustfmt two pre-existing files in lakesoul-datafusion. Covered by tests/insert_contract.rs against a real PostgreSQL metadata database, including authoritative snapshot checks around every refusal. Verified on PostgreSQL 17.10 in both merge configurations: eight targets 20 passed/1 ignored each, --bins 123 passed/1 ignored, rustfmt clean, metadata residue zero. Co-Authored-By: DeepSeek V4 Flash <[email protected]> Change-Id: Icaae5770900ea866036e8bec268b04196a6a6964 Co-Authored-By: Codex <[email protected]> Signed-off-by: mag1cian <[email protected]> * fix(pg): encode zero NUMERIC with the column scale Zero DECIMAL values lost their scale on the extended-protocol binary path: rust_decimal serializes zero with dscale 0, while PostgreSQL keeps the column scale (0.00 decodes as scale 2). Re-encode top-level Decimal128 zero cells in the query response stream using the statement's Arrow schema before they reach the client; nonzero cells stay byte-identical to the upstream encoder. Decimal256 results and negative DECIMAL scales are rejected with 0A000 instead of being truncated, and a scale beyond PostgreSQL's 14-bit dscale is 22003. The strict reproduction becomes a normal gate and now diffs 0, 0.00, 1e-20 and -0.00 against PostgreSQL; sql_compat no longer has an ignored case. Verified on PostgreSQL 17.10 in both merge configurations: sql_compat 5 passed/0 ignored, --bins 127 passed/1 ignored (the remaining ignored case is the unrelated routing timezone test), rustfmt and scoped clippy clean. Co-Authored-By: DeepSeek V4 Flash <[email protected]> Change-Id: If0bd4670e9b387ce6a529aad57fcd5626a6a6964 Co-Authored-By: Codex <[email protected]> Signed-off-by: mag1cian <[email protected]> * fix(pg): report 25P02 for statements in a failed transaction PostgreSQL ignores statements in a failed transaction block with SQLSTATE 25P02 (in_failed_sql_transaction); the upstream transaction hook reports 25P01 instead. Add an in-repo guard immediately in front of that hook: it reads the same per-connection transaction status and answers first for simple queries, extended Parse and Execute of statements prepared while the block was healthy. COMMIT and ROLLBACK still pass through, so a failed block ends as PostgreSQL ends it (COMMIT is accepted and tagged ROLLBACK). The two failed-transaction contracts become differential against PostgreSQL, asserting the same 25P02 for the same SQL on both sides and covering COMMIT-in-failed-block; the state is read from the connection, never matched on error text. Verified on PostgreSQL 17.10 in both merge configurations: session_contract 3 passed/0 ignored, sql_compat 5 passed/0 ignored, --bins 127 passed/1 ignored (unrelated routing timezone case), rustfmt and scoped clippy clean. Co-Authored-By: DeepSeek V4 Flash <[email protected]> Change-Id: I5b5e85e3f61ca40dc1aa882e1500ca226a6a6964 Co-Authored-By: Codex <[email protected]> Signed-off-by: mag1cian <[email protected]> * fix(pg): invalidate prepared plans after own DDL; document catalog binding A prepared statement binds its TableProvider at Parse. A successful CREATE TABLE or DROP TABLE on the same connection now advances the connection's settings generation, so the next Execute replans: after the connection drops a table the bound scan reports 42P01 like PostgreSQL instead of the generic catalog-resolution XX000, and DROP + same-name CREATE binds the new table. DDL performed by another connection does not invalidate this connection's plans: the old binding keeps executing (zero rows for a dropped table, and a prepared INSERT into a dropped table fails with 42P01 and leaves no metadata or file residue). That is a documented LakeSoul catalog-binding boundary, not PostgreSQL equivalence. Committed data still needs no re-Prepare: the physical scan re-lists files on every Execute, as read-committed-first-select already proves. Tests: same-connection DDL replans a prepared statement, cross-connection drop and drop+recreate keep the bound plan, prepared INSERT into a dropped table leaves no state. Verified on PostgreSQL 17.10 in both merge configurations: ddl_contract 2 passed, insert_contract 2 passed, session_contract 3 passed, sql_compat 5 passed/0 ignored, --bins 127 passed/1 ignored, rustfmt and scoped clippy clean. Co-Authored-By: DeepSeek V4 Flash <[email protected]> Change-Id: I22e66f0de7857d6241c64029dbee00446a6a6964 Co-Authored-By: Codex <[email protected]> Signed-off-by: mag1cian <[email protected]> * fix(pg): analyze INSERT like PostgreSQL for 42601/42804/22P02 An INSERT is analysed before planning, the way PostgreSQL analyses it. The source width must match the exported columns: too many expressions is 42601, and an explicit column list with too few expressions is 42601; a no-list source shorter than the table is completed with NULLs for the unlisted columns, in VALUES rows and in SELECT projections, which is PostgreSQL's default semantics for those columns. An untyped string literal is converted against the target column (22P02 when it does not convert), while every other expression must be assignment-castable (42804) under a matrix derived from PostgreSQL 17.10: numeric families interchange, text accepts every type, temporal types interchange, and boolean and binary stay within their own family. `SELECT` expression casts keep their documented XX000 boundary, and a wildcard projection is checked through the planned source type because positions cannot be mapped from the AST. Tests: differential cases against the reference PostgreSQL for 22P02, 42804 and 42601 in both protocols, the NULL-fill success paths for VALUES and SELECT, a mixed literal/column SELECT, and a control insert; every rejection keeps the authoritative snapshot unchanged. Verified on PostgreSQL 17.10 in both merge configurations: insert_contract 2 passed, sql_compat 5 passed/0 ignored, ddl_contract 2 passed, session_contract 3 passed, --bins 127 passed/1 ignored, rustfmt and scoped clippy clean. Signed-off-by: mag1cian <[email protected]> Co-Authored-By: DeepSeek V4 Flash <[email protected]> Change-Id: I8d05f208eef2cedbd147ac08bff8d6706a6a6964 Co-Authored-By: Codex <[email protected]> Signed-off-by: mag1cian <[email protected]> * fix(pg): reject duplicate upsert keys with 21000 An `INSERT ... ON CONFLICT DO UPDATE` whose source produces the same primary key twice is now rejected with PostgreSQL's cardinality violation (21000, "cannot affect row a second time"). The statement's source is grouped by key before the write; a duplicated key fails the statement before any file is written or metadata published, so a rejected upsert leaves no state behind. The check runs at Execute in both protocols, because parameters must be bound before the source can be evaluated. `ON CONFLICT DO NOTHING` still drops the duplicate instead of failing, a plain `INSERT` still reports 23505 from the sink, and distinct keys still replace as before. Tests: duplicate source in both protocols against the reference PostgreSQL (21000 on both), authoritative snapshot unchanged, DO NOTHING accepted with one surviving row, and a distinct-key control that updates an existing row and inserts a new one. Verified on PostgreSQL 17.10 in both merge configurations: insert_contract 2 passed, sql_compat 5 passed/0 ignored, ddl_contract 2 passed, session_contract 3 passed, --bins 127 passed/1 ignored, rustfmt and scoped clippy clean. Co-Authored-By: DeepSeek V4 Flash <[email protected]> Change-Id: Ibde0ea1390534716531a5990eeddcf576a6a6964 Co-Authored-By: Codex <[email protected]> Signed-off-by: mag1cian <[email protected]> * [PG]: rename hash_bucket_num table option `CREATE TABLE ... WITH (hashbucketnum = N)` stored the bucket count but the spelling was inconsistent with the other `WITH` options (`file_format`, `use_cdc`, `cdc_column`). Rename the PG-facing option to `hash_bucket_num`. - `ddl.rs`: match `hash_bucket_num` in `WITH` options and in the default injection key. The injection key must move in lockstep: leaving the old spelling as the default would overwrite a user-supplied `hash_bucket_num` with the provider default (`table_provider.rs` reads `hash_bucket_num` before `format.hash_bucket_num`). - Old spellings `hashbucketnum` and `hashBucketNum` are rejected `0A000` (no alias shim); out-of-range values remain `22023`. - Metadata property stays `hashBucketNum` (Spark/Flink persisted key); no storage migration. Contract tests: `options_and_column_types` asserts a non-default bucket count is stored; the refusal matrix and extended-protocol rejection list cover the old spellings; zero/negative/bound tests use the new spelling. Co-Authored-By: DeepSeek V4 Flash <[email protected]> Change-Id: Ie093110c9c0a1774f66b159c158fd64f6a6a6964 Co-Authored-By: Codex <[email protected]> Signed-off-by: mag1cian <[email protected]> * [PG]: default file_format to vortex-compact `CREATE TABLE` without `WITH (file_format = ...)` previously defaulted to `parquet`; the B7 product decision is `vortex-compact`. The write path already supported it (`writer/mod.rs` dispatches `PhysicalFormat::VortexCompact` to `VortexSink::new(..., compact=true)` = ZSTD) and the read path auto-detects `.vortex`, so the default is a one-line injection change. - `ddl.rs`: default injection key now `vortex-compact`. - `ddl_contract::create_table_publishes_metadata`: asserts the default is `vortex-compact`; `options_and_column_types` keeps an explicit `file_format = 'parquet'` to prove the default does not override an explicit choice. - `insert_contract` wording no longer names parquet for the default-format in-test tables. - README stage-7 semantics reflect the new default. Verified: legacy and v2 merge `ddl_contract` + `insert_contract` pass; end-to-end smoke writes real `.vortex` files (metadata `vortex-compact`) with exact 96-row readback, and explicit `parquet`/`vortex`/`vortex-compact` each produce their own layout. Co-Authored-By: DeepSeek V4 Flash <[email protected]> Change-Id: I956d3065f07ae6633657e43093528eb36a6a6964 Co-Authored-By: Codex <[email protected]> Signed-off-by: mag1cian <[email protected]> * [PG]: accept append_only table option `CREATE TABLE ... WITH (append_only = true|false)` now persists the Spark/Flink `appendOnly` table property. The value is validated at the PG boundary (case-insensitive `true`/`false`, normalised to lowercase, invalid `22023`) and also accepts a bare boolean literal. Persistence goes through the typed property struct: `LakeSoulTableProperty` gains an `append_only` field (`serde(rename = "appendOnly")`), and `table_provider` maps `format.append_only`/`append_only` into it. Without both, `serde_json::to_string` would silently drop the option. The PG layer does not enforce the property yet: `UPDATE`/`DELETE` remain `0A000`, and the Rust read/write paths do not consult `appendOnly`. The README states it is persisted but not yet enforced. Verified: legacy and v2 merge `ddl_contract` pass (explicit `append_only = true`, quoted `FALSE`, invalid `yes` -> 22023, extended protocol), and `insert_contract` + bins pass under legacy. Co-Authored-By: DeepSeek V4 Flash <[email protected]> Change-Id: I868037803deca51bc48764e2a39ff1856a6a6964 Co-Authored-By: Codex <[email protected]> Signed-off-by: mag1cian <[email protected]> * [PG]: accept use_cdc and lakesoul_cdc_change_column options `CREATE TABLE ... WITH (use_cdc = true|false, lakesoul_cdc_change_column = 'col')` now persists the two LakeSoul CDC properties. Both are live in this front end, unlike `append_only`: - `use_cdc` enables `stable_sort` on write (input order decides the newest version of a repeated key). - `lakesoul_cdc_change_column` enables the read-side tombstone filter: `col != 'delete'` is applied after merge-on-read, so a delete marker is the newest version of a key, participates in the merge, and is then dropped. A later insert revives the key. At CREATE TABLE the change column is resolved against the declared schema: it must name a declared column (`42703` otherwise), and `use_cdc = true` without an explicit column falls back to `rowKinds`, which must also be declared (the front end folds names, so the property stores `rowkinds`). The PG layer never injects a hidden column into the schema. `UPDATE`/`DELETE` statements remain `0A000`: CDC here is data-driven, the user writes the change-column value. Verified: legacy and v2 merge `ddl_contract` + `insert_contract` pass, including the tombstone/revival case. Co-Authored-By: DeepSeek V4 Flash <[email protected]> Change-Id: Ia3b1f2e1977be54b64cd485bfbf8de226a6a6964 Co-Authored-By: Codex <[email protected]> Signed-off-by: mag1cian <[email protected]> * benchmark: apply treefmt formatting Format the PostgreSQL benchmark driver, scripts, TPC-H answer generator, workload mixes, the TPC-H compatibility workflow, and the lakesoul-io manifest with the repository formatter. Python changes are AST-identical; this only satisfies the Format Check CI gate. Exclude the unrendered console-job.yaml template from prettier: its bare ${EXTRA_ENV} placeholder is substituted with an indented env block by scripts/bench.py, so the template is not valid YAML on its own. Co-Authored-By: Codex <[email protected]> Signed-off-by: mag1cian <[email protected]> * fix: skip metadata publication when no files were written The multi-partition publish path resolves the table id up front so one transaction can publish every range partition. A statement that wrote no files (zero-row INSERT, or an empty sink) has nothing to publish, but it still resolved the table and failed with `table ... not found` when the name was not in the metadata store. Upstream main iterated the partitions and committed nothing in that case. Resolve and publish only when at least one partition has files, restoring the no-op behaviour; the commit metric still finalizes on the same path. Co-Authored-By: Codex <[email protected]> Signed-off-by: mag1cian <[email protected]> * fix: keep the merge memory consumer for v2 merges v2 merges run the merger on a spawned task, and the plan-level LakeSoulMerge consumer was dropped as soon as the stream was built, so a lane held no consumer while its merge was open: memory accounting missed the merge and the one-merge-per-lane guard saw nothing. Thread the reservation through create_merger! and have WindowSlidingMerger hold it for its task lifetime. A v2 unit can still finish and release before its first batch is read (the task buffers the unit), so the lane test now asserts the durable invariants: one merge opened, none for later units, and release after the lane is dropped (awaited, because the task is aborted asynchronously). Verified: physical_plan::merge tests 27/27 under both merge modes; lakesoul-io lib 190 passed / 1 ignored under both modes; clippy -D warnings on lakesoul-io. Co-Authored-By: Codex <[email protected]> Signed-off-by: mag1cian <[email protected]> --------- Signed-off-by: mag1cian <[email protected]> Co-authored-by: Claude Opus 5.5 <[email protected]> Co-authored-by: Codex <[email protected]> Co-authored-by: DeepSeek V4 Flash <[email protected]>

apache#24891) Closes apache#24883 Under a memory limit, a grouped aggregation on two or more partitions can stop and never complete: when `RepartitionExec` cannot reserve memory for a batch it writes the batch to its spill pool and sends a marker, and the reader then blocks on the pool until it gets a batch. When two input tasks spill at the same time the pool holds two open files. The reader read only the oldest file and, when that file had no unread batch, waited for it, while the batch it needed was in the newer file. Once every channel had data the gate closed and the writers waited too, so nothing could wake the reader. `SpillPoolReader` now keeps every file it received (oldest first) and returns the first available batch. A file that has no unread batch is skipped; the reader still waits on the oldest file that has unread batches whose read is not complete, so a single writer keeps FIFO order. The writer side is unchanged. Also backported: - `test: reproduce the RepartitionExec spill pool deadlock with a SQL query` (`datafusion/core/tests/memory_limit/repartition_mem_limit.rs`), which sums the `spill_count` metric and fails if no attempt spilled. - `test: add a scenario fuzzer for the spill pool contract`, including `test_read_error_is_reported_to_the_reader`, a test that a second writer's batch is readable while the first writer is held, and `spill_pool_scenario_fuzz` (default 50 seeds). - the extended-CI job sets `DATAFUSION_SPILL_POOL_FUZZ_ITERATIONS: 1000`. Backport notes: applied by hand because `git cherry-pick -x` is not usable here (the worktree's git directory is not writable from this sandbox). All three files applied byte-identically; the only fork-specific deltas in the touched regions are pre-existing cosmetic differences (`#[deprecated]` without `since`, `sort_unstable` vs `sort`), which this commit does not touch. (cherry picked from commit 09a2aff) (backport of apache#24891, 09a2aff) Signed-off-by: discord9 <[email protected]> (cherry picked from commit ac60580950ee42b7358a74fdf1203a0dc5e84992)

adriangb requested a balanced review from Copilot September 2, 2026 21:29

github-actions Bot added development-process Related to development process of DataFusion core Core DataFusion crate sqllogictest SQL Logic Tests (.slt) physical-plan Changes to the physical-plan crate labels Sep 2, 2026

Copilot started reviewing on behalf of adriangb September 2, 2026 21:30

View session

adriangb commented Sep 2, 2026

View reviewed changes

Comment thread datafusion/sqllogictest/test_files/repartition_memory_spill.slt Resolved

Comment thread datafusion/sqllogictest/test_files/repartition_memory_spill.slt Resolved

Copilot AI reviewed Sep 2, 2026

View reviewed changes

Comment thread datafusion/physical-plan/src/spill/spill_pool.rs Resolved

github-actions Bot removed the sqllogictest SQL Logic Tests (.slt) label Sep 2, 2026

adriangb force-pushed the claude/datafusion-24883-fixes-jeha12 branch from 0776f74 to bd1c0c0 Compare September 2, 2026 22:54

adriangb requested a balanced review from Copilot September 3, 2026 05:23

Copilot started reviewing on behalf of adriangb September 3, 2026 05:23

View session

Copilot AI reviewed Sep 3, 2026

View reviewed changes

Comment thread datafusion/physical-plan/src/spill/spill_pool.rs Resolved

adriangb force-pushed the claude/datafusion-24883-fixes-jeha12 branch from bd1c0c0 to eac4063 Compare September 3, 2026 05:41

claude added 3 commits September 3, 2026 15:02

adriangb force-pushed the claude/datafusion-24883-fixes-jeha12 branch from eac4063 to 7b6bc88 Compare September 3, 2026 15:08

adriangb requested review from 2010YOUY01 and kosiew September 3, 2026 15:11

kosiew approved these changes Sep 4, 2026

View reviewed changes

adriangb enabled auto-merge September 4, 2026 15:38

adriangb added this pull request to the merge queue Sep 4, 2026

Merged via the queue into apache:main with commit 09a2aff Sep 4, 2026
41 checks passed

adriangb deleted the claude/datafusion-24883-fixes-jeha12 branch September 4, 2026 16:16

alamb mentioned this pull request Sep 25, 2026

Release DataFusion 55.2.0 (patch) Release (Oct 2026) #25758

Closed

6 of 21 tasks

nuno-faria added the v56.0.0 label Oct 3, 2026

DecisionNerd mentioned this pull request Oct 13, 2026

chore(deps): qualify and migrate to stock DataFusion 56 CurateLabs/graphforge#2042

Open

9 tasks

DecisionNerd mentioned this pull request Oct 13, 2026

fix(exec): backport the DataFusion spill-pool deadlock fix (apache/datafusion#24891) into a vendored datafusion-physical-plan 54.1.0 (Part of #1985) CurateLabs/graphforge#2046

Merged

wombatu-kun mentioned this pull request Nov 2, 2026

bug: indexed merge_insert fails or hangs under a bounded memory pool lance-format/lance#10580

Closed

discord9 mentioned this pull request Nov 9, 2026

fix: backport null-aware anti-join, timestamp-to-time pruning and spill fixes GreptimeTeam/datafusion#42

Merged

discord9 mentioned this pull request Nov 10, 2026

feat: Add BatchIterator trait and support iterating btree memtable GreptimeTeam/greptimedb#42

Merged

Navigation Menu

Repository navigation

fix: RepartitionExec deadlock when its spill pool holds two open files - #24891

fix: RepartitionExec deadlock when its spill pool holds two open files#24891
adriangb merged 4 commits into
apache:mainfrom
pydantic:claude/datafusion-24883-fixes-jeha12

adriangb commented Sep 2, 2026 •

edited

Loading

Uh oh!

adriangb left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

codecov-commenter commented Sep 2, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

adriangb commented Sep 3, 2026

Uh oh!

kosiew left a comment •

edited

Loading

Uh oh!

kosiew Sep 4, 2026

Uh oh!

adriangb Sep 4, 2026

Uh oh!

adriangb commented Sep 4, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

adriangb commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

What is the testing strategy for this PR?

Are there any user-facing changes?

Uh oh!

adriangb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Uh oh!

codecov-commenter commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Suppressed comments (1)

Uh oh!

Uh oh!

adriangb commented Sep 3, 2026

Uh oh!

kosiew left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kosiew Sep 4, 2026

Choose a reason for hiding this comment

Uh oh!

adriangb Sep 4, 2026

Choose a reason for hiding this comment

Uh oh!

adriangb commented Sep 4, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

adriangb commented Sep 2, 2026 •

edited

Loading

codecov-commenter commented Sep 2, 2026 •

edited

Loading

kosiew left a comment •

edited

Loading