Conversation
deniskuzZ
force-pushed
the
mapjoin-key-sizing
branch
from
September 19, 2026 10:58
be74fde to
1f888d6
Compare
deniskuzZ
force-pushed
the
mapjoin-key-sizing
branch
2 times, most recently
from
September 19, 2026 12:00
c1d1407 to
c88f9fc
Compare
asf-ci-hive
added
tests pending
tests passed
and removed
tests failed
tests pending
labels
deniskuzZ
force-pushed
the
mapjoin-key-sizing
branch
from
September 22, 2026 20:42
c88f9fc to
eb40d07
Compare
deniskuzZ
force-pushed
the
mapjoin-key-sizing
branch
from
September 22, 2026 20:55
eb40d07 to
2035a69
Compare
asf-ci-hive
added
tests failed
tests pending
and removed
tests pending
tests failed
labels
deniskuzZ
force-pushed
the
mapjoin-key-sizing
branch
from
September 24, 2026 11:43
2035a69 to
dee3089
Compare
deniskuzZ
force-pushed
the
mapjoin-key-sizing
branch
2 times, most recently
from
September 24, 2026 14:01
5d36fe2 to
627f6e9
Compare
deniskuzZ
changed the title
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
One or more issues must be addressed before approval.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
This PR improves map-join hash-table sizing and prevents publishing partially loaded tables after drain-thread failures.
Changes:
- Size tables using the smaller positive key estimate or input-record signal.
- Propagate drain-thread failures before sealing/publishing tables.
- Add unit and integration-style tests for sizing and failure handling.
| File | Description |
|---|---|
| ql/src/test/org/apache/hadoop/hive/ql/exec/vector/mapjoin/fast/TestVectorMapJoinFastHashTableLoaderDrain.java | Updated as part of this pull request. |
| ql/src/test/org/apache/hadoop/hive/ql/exec/vector/mapjoin/fast/TestVectorMapJoinFastHashTableLoader.java | Updated as part of this pull request. |
| ql/src/test/org/apache/hadoop/hive/ql/exec/TestHashTableLoaderKeyCountForSizing.java | Updated as part of this pull request. |
| ql/src/java/org/apache/hadoop/hive/ql/exec/vector/mapjoin/fast/VectorMapJoinFastHashTableLoader.java | Updated as part of this pull request. |
| ql/src/java/org/apache/hadoop/hive/ql/exec/tez/HashTableLoader.java | Updated as part of this pull request. |
| ql/src/java/org/apache/hadoop/hive/ql/exec/HashTableLoader.java | Updated as part of this pull request. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+20
to
+31
| import java.io.IOException; | ||
| import java.util.ArrayList; | ||
| import java.util.HashMap; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.concurrent.ExecutorService; | ||
| import java.util.concurrent.Executors; | ||
| import java.util.concurrent.TimeUnit; | ||
| import java.util.concurrent.atomic.AtomicInteger; | ||
| import java.util.concurrent.atomic.AtomicReference; | ||
|
|
||
| import com.google.common.util.concurrent.ThreadFactoryBuilder; |
|
- HIVE-23953 sized from max(estimate, APPROXIMATE_INPUT_RECORDS). Either signal can be wrong. Sizing high allocates the slot arrays before the first row, resident memory the monitor only sees afterwards. Sizing low costs a slot-array copy per doubling (about one final size of garbage in total), an old+new transient the monitor never samples, one load thread below FIRST_SIZE_UP, and collision-triggered resizes below it. The first is a dead query, the rest are time; take the min. A row count bounds a key count from above, never from below - q23 at 10Tb, 10 x r5d, stock Tez, exhibits from the c88f9fc image (min sizing, thread count still from max; Map 10 gets 2 threads under both rules). Map 10: estimate 65,000,000, counter 320,243,730. max allocates 8,589,967,648 bytes and dies on MapJoinMemoryExhaustionError at the first memory check; min allocates 2,147,516,704 and loads 65M rows in 10 s. Reducer 9: estimate 21,666,666 for 17,975 rows, counter 18,166; the estimate alone allocates 512 MiB per task, min 0.5 MiB (2 tables there, 1 under this commit, same bytes). Over the query's 80 loads min rehashes none and over-allocates none - TEZ-4757 inflates the counter 4.93x on Map 10; independent fix, and a correct counter is still a row count - With neither signal the rule yields -1, not 0: calculateTableSize honours 0 as a real size - numLoadThreads divides the sized capacity, so it reads the same count - Read the drain-thread futures back; a thread that threw still left the executor terminated, so the table was sealed with one partition's rows missing. An Error is rethrown unwrapped
deniskuzZ
force-pushed
the
mapjoin-key-sizing
branch
from
September 25, 2026 10:21
627f6e9 to
c49ade4
Compare
asf-ci-hive
added
tests pending
tests failed
and removed
tests unstable
tests pending
labels
This branch has not been deployed
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.
What changes were proposed in this pull request?
Why are the changes needed?
MapJoinMemoryExhaustionError: input: Map 10
Does this PR introduce any user-facing change?
No
How was this patch tested?
TestHashTableLoaderKeyCountForSizing, TestVectorMapJoinFastHashTableLoader, TestVectorMapJoinFastHashTableLoaderDrain
Cluster 10Tb Parquet & Orc