nsivabalan opened a new pull request, #19085:
URL: https://github.com/apache/hudi/pull/19085

   ### Change Logs
   
   Fixes apache/hudi#19084.
   
   MDT `record_index` partition compaction OOMs executors configured with 9 GB 
heap, four concurrent tasks, and `spark.memory.fraction=0.3`. Heap analysis 
identified three structural consumers that aren't bounded by the configured 
compaction memory cap (`hoodie.memory.compaction.fraction`):
   
   | Consumer | What | Total (4 tasks) |
   |---|---|---|
   | B | `BitCaskDiskMap.valueMetadataMap` — 18.3M `ValueMetadata` entries with 
`Integer`/`Long` boxing overhead dominating (57.2M `java.lang.Long` instances = 
~1.28 GB pure boxing) | ~3.0–3.2 GB |
   | F | `LazyFileIterator` materialised a full sorted `ArrayList` (4.58M 
entries × 4 tasks) at iterator-open time because `ConcurrentHashMap` doesn't 
preserve insertion order | ~513 MB |
   | E | `ExternalSpillableMap.keySet()` allocated a transient `HashSet` 
copying all 6.29M keys solely to seed the `PriorityQueue` in 
`HoodieSortedMergeHandle`'s constructor | ~256 MB peak |
   
   4-task demand: ~6.25 GB on a 9 GB executor — OOMs under GC pressure. 
Lowering `hoodie.memory.compaction.fraction` makes B larger and the OOM worse, 
so the fix must be code, not config.
   
   ### Changes
   
   1. **Unbox `ValueMetadata` primitives** (`BitCaskDiskMap.java`) — 
`Integer`/`Long` → `int`/`long` for `sizeOfValue`, `offsetOfValue`, 
`timestamp`. Saves ~880 MB across 4 tasks. The Lombok `@AllArgsConstructor` + 
`@Getter` annotations are preserved; getter return types switch from boxed to 
primitive (autoboxing handles any external caller).
   
   2. **`LinkedHashMap` + `ReentrantReadWriteLock` for `valueMetadataMap`** 
(`BitCaskDiskMap.java`, `LazyFileIterable.java`) — `filePosition` is strictly 
monotonic under the write lock (append-only file + monotonic 
`SizeAwareDataOutputStream.getSize()`), so insertion order = disk offset order. 
`LazyFileIterator` no longer sorts; it consumes a pre-built snapshot list taken 
under the read lock. `put()` does remove-before-put under the write lock so 
re-inserted keys land at the `LinkedHashMap` tail. All read paths (`get`, 
`containsKey`, `size`, `iterator`, `valueStream`, `entrySet`, `keySet`, 
`iterator(Predicate)`) take the read lock and snapshot before releasing. 
`close()` and `clear()` take the write lock so a concurrent reader under 
`SpillableMapBasedFileSystemView`'s shared `readLock` can't observe a 
half-cleared map. Saves ~513 MB across 4 tasks.
   
   3. **`ExternalSpillableMap.keyStream()` + `HoodieSortedMergeHandle` uses 
it** (`ExternalSpillableMap.java`, `HoodieSortedMergeHandle.java`) — 
`keyStream()` returns `Stream.concat(inMemoryKeys, diskKeys)` without 
allocating a `HashSet` copy. `HoodieSortedMergeHandle` now collects the stream 
into an `ArrayList` and passes it to `new PriorityQueue<>(list)` to get the 
O(N) heapify (vs. O(N log N) per-add). Saves ~256 MB peak spike per task.
   
   Total: ~1.65 GB per task. 4-task demand drops from ~6.25 GB to ~4.57 GB.
   
   ### Impact
   
   Eliminates executor OOM during MDT `record_index` compaction on 
commonly-sized executors. No on-disk format change. No public-API surface 
change (`ValueMetadata.getOffsetOfValue()` return type changes from `Long` to 
`long`, but `ValueMetadata` is an internal class and autoboxing handles any 
external callers).
   
   ### Risk Level
   
   Low. The locking model is conservative (RW-lock with read-shared concurrency 
preserved), the insertion-order = offset-order invariant is proved from 
append-only file + monotonic `filePosition` + write-lock, and the regression 
test suite locks down both the invariant and the concurrent-read contract.
   
   ### Documentation Update
   
   None required.
   
   ### Tests
   
   - `TestBitCaskDiskMap#testReInsertionPreservesOffsetOrder` (parameterized × 
compression on/off): puts a key twice, asserts re-inserted key lands at 
iteration tail. Locks down the `LinkedHashMap`-insertion-order == 
disk-offset-order invariant against future refactors of `put()`.
   - `TestBitCaskDiskMap#testConcurrentReadersAndWriter`: writer + reader 
threads; asserts no `ConcurrentModificationException` and that the RW-lock 
contract holds.
   - Full `TestBitCaskDiskMap` + `TestExternalSpillableMap` suite: **45 run / 0 
failed / 1 skipped** on Java 11 with `-Dscala-2.12 -Dspark3.5`.
   
   ### Contributor's checklist
   
   - [x] Read through [contributor's 
guide](https://hudi.apache.org/contribute/how-to-contribute)
   - [x] Change Logs and Impact were stated clearly
   - [x] Adequate tests were added if applicable
   - [ ] CI passed


-- 
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]

Reply via email to