sunchao opened a new pull request, #25508:
URL: https://github.com/apache/datafusion/pull/25508

   ## Which issue does this PR close?
   
   Addresses the shared accounting bug identified while reviewing 
[#25491](https://github.com/apache/datafusion/pull/25491#discussion_r4052388920).
 Split out so it can be reviewed and backported independently of prepared-build 
reuse.
   
   ## Rationale for this change
   
   Hash joins reserve memory for their hash buckets, but omit the row-index 
vector used to link build rows with the same key. As a result, a join can 
allocate beyond its admitted budget and underreport its build memory. For 
example, 100 million build rows need an additional 400 MB for this vector that 
the memory pool currently does not see.
   
   ## What changes are included in this PR?
   
   The shared hash-map constructor now reserves the bucket estimate and 
row-index storage together before allocating either, and includes both in the 
build-memory metric. This covers ordinary CollectLeft and partitioned joins, 
using four bytes per row for the u32 representation and eight for u64. 
Perfect-hash array maps keep their existing accounting.
   
   ## What is the testing strategy for this PR?
   
   A regression gives the constructor one byte less than the complete budget 
and checks that it refuses the allocation without charging the pool or metric. 
It also verifies success at the exact limit and release on drop. The test fails 
against the original constructor and passes with this fix.
   
   All 533 hash-join tests, full workspace Clippy with all targets/features and 
warnings denied, formatting, and whitespace checks pass on this branch. The 
combined prepared-build branch also passed the extended workspace run with this 
constructor fix (11,995 Rust tests and 521 SQL test files; eight tests ignored).
   
   Local checks used Rust 1.98.1 and temporary source overrides for the 
requested public releases of Arrow 60.0.0, object_store 0.14.2, sqlparser 
0.63.0, and compression-codecs 0.4.42 because the registry mirror lacked them. 
No dependency or lockfile changes are included. Ordinary-path benchmark 
comparisons for the combined branch, including the accounting fix, are reported 
in #25491; they are not an isolated performance claim for this fix.
   
   ## Are there any user-facing changes?
   
   Hash joins using these maps account for their row-index storage in memory 
limits and build-memory metrics. A budget that previously succeeded only 
because this allocation was omitted can now be rejected before allocation.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to