Skip to content

Commit b2baaa2

Browse files
viiryadongjoon-hyun
authored andcommitted
[SPARK-30274][CORE] Avoid BytesToBytesMap lookup hang forever when holding keys reaching max capacity
### What changes were proposed in this pull request? We should not append keys to BytesToBytesMap to be its max capacity. ### Why are the changes needed? BytesToBytesMap.append allows to append keys until the number of keys reaches MAX_CAPACITY. But once the the pointer array in the map holds MAX_CAPACITY keys, next time call of lookup will hang forever. ### Does this PR introduce any user-facing change? No ### How was this patch tested? Manually test by: ```java Test public void testCapacity() { TestMemoryManager memoryManager2 = new TestMemoryManager( new SparkConf() .set(package$.MODULE$.MEMORY_OFFHEAP_ENABLED(), true) .set(package$.MODULE$.MEMORY_OFFHEAP_SIZE(), 25600 * 1024 * 1024L) .set(package$.MODULE$.SHUFFLE_SPILL_COMPRESS(), false) .set(package$.MODULE$.SHUFFLE_COMPRESS(), false)); TaskMemoryManager taskMemoryManager2 = new TaskMemoryManager(memoryManager2, 0); final long pageSizeBytes = 8000000 + 8; // 8 bytes for end-of-page marker final BytesToBytesMap map = new BytesToBytesMap(taskMemoryManager2, 1024, pageSizeBytes); try { for (long i = 0; i < BytesToBytesMap.MAX_CAPACITY + 1; i++) { final long[] value = new long[]{i}; boolean succeed = map.lookup(value, Platform.LONG_ARRAY_OFFSET, 8).append( value, Platform.LONG_ARRAY_OFFSET, 8, value, Platform.LONG_ARRAY_OFFSET, 8); } map.free(); } finally { map.free(); } } ``` Once the map was appended to 536870912 keys (MAX_CAPACITY), the next lookup will hang. Closes #26914 from viirya/fix-bytemap2. Authored-by: Liang-Chi Hsieh <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>
1 parent cdc8fc6 commit b2baaa2

File tree

1 file changed

+4
-1
lines changed

1 file changed

+4
-1
lines changed

core/src/main/java/org/apache/spark/unsafe/map/BytesToBytesMap.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -694,7 +694,10 @@ public boolean append(Object kbase, long koff, int klen, Object vbase, long voff
694694
assert (vlen % 8 == 0);
695695
assert (longArray != null);
696696

697-
if (numKeys == MAX_CAPACITY
697+
// We should not increase number of keys to be MAX_CAPACITY. The usage pattern of this map is
698+
// lookup + append. If we append key until the number of keys to be MAX_CAPACITY, next time
699+
// the call of lookup will hang forever because it cannot find an empty slot.
700+
if (numKeys == MAX_CAPACITY - 1
698701
// The map could be reused from last spill (because of no enough memory to grow),
699702
// then we don't try to grow again if hit the `growthThreshold`.
700703
|| !canGrowArray && numKeys >= growthThreshold) {

0 commit comments

Comments
 (0)