Skip to content

Commit faf8dd5

Browse files
advancedxysrowen
authored andcommitted
[SPARK-33756][SQL] Make BytesToBytesMap's MapIterator idempotent
### What changes were proposed in this pull request? Make MapIterator of BytesToBytesMap `hasNext` method idempotent ### Why are the changes needed? The `hasNext` maybe called multiple times, if not guarded, second call of hasNext method after reaching the end of iterator will throw NoSuchElement exception. ### Does this PR introduce _any_ user-facing change? NO. ### How was this patch tested? Update a unit test to cover this case. Closes #30728 from advancedxy/SPARK-33756. Authored-by: Xianjin YE <[email protected]> Signed-off-by: Sean Owen <[email protected]> (cherry picked from commit 1339168) Signed-off-by: Sean Owen <[email protected]>
1 parent 7881622 commit faf8dd5

File tree

2 files changed

+8
-4
lines changed

2 files changed

+8
-4
lines changed

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -393,10 +393,12 @@ public void remove() {
393393
}
394394

395395
private void handleFailedDelete() {
396-
// remove the spill file from disk
397-
File file = spillWriters.removeFirst().getFile();
398-
if (file != null && file.exists() && !file.delete()) {
399-
logger.error("Was unable to delete spill file {}", file.getAbsolutePath());
396+
if (spillWriters.size() > 0) {
397+
// remove the spill file from disk
398+
File file = spillWriters.removeFirst().getFile();
399+
if (file != null && file.exists() && !file.delete()) {
400+
logger.error("Was unable to delete spill file {}", file.getAbsolutePath());
401+
}
400402
}
401403
}
402404
}

core/src/test/java/org/apache/spark/unsafe/map/AbstractBytesToBytesMapSuite.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -561,6 +561,8 @@ public void spillInIterator() throws IOException {
561561
iter2.next();
562562
}
563563
assertFalse(iter2.hasNext());
564+
// calls hasNext twice deliberately, make sure it's idempotent
565+
assertFalse(iter2.hasNext());
564566
} finally {
565567
map.free();
566568
for (File spillFile : spillFilesCreated) {

0 commit comments

Comments
 (0)