[spark] branch branch-2.4 updated: [SPARK-33756][SQL] Make BytesToBytesMap's MapIterator idempotent

[srowen]

This is an automated email from the ASF dual-hosted git repository.

srowen pushed a commit to branch branch-2.4
in repository https://gitbox.apache.org/repos/asf/spark.git


The following commit(s) were added to refs/heads/branch-2.4 by this push:
     new 9b8c193  [SPARK-33756][SQL] Make BytesToBytesMap's MapIterator 
idempotent
9b8c193 is described below

commit 9b8c1930ed57117a70310d66eb1ab44f7ece9e0d
Author: Xianjin YE <advance...@gmail.com>
AuthorDate: Sun Dec 20 08:51:17 2020 -0600

    [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 <advance...@gmail.com>
    Signed-off-by: Sean Owen <sro...@gmail.com>
    (cherry picked from commit 13391683e7a863671d3d719dc81e20ec2a870725)
    Signed-off-by: Sean Owen <sro...@gmail.com>
---
 .../main/java/org/apache/spark/unsafe/map/BytesToBytesMap.java | 10 ++++++----
 .../apache/spark/unsafe/map/AbstractBytesToBytesMapSuite.java  |  2 ++
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git 
a/core/src/main/java/org/apache/spark/unsafe/map/BytesToBytesMap.java 
b/core/src/main/java/org/apache/spark/unsafe/map/BytesToBytesMap.java
index e2d258a..b38f845 100644
--- a/core/src/main/java/org/apache/spark/unsafe/map/BytesToBytesMap.java
+++ b/core/src/main/java/org/apache/spark/unsafe/map/BytesToBytesMap.java
@@ -408,10 +408,12 @@ public final class BytesToBytesMap extends MemoryConsumer 
{
     }
 
     private void handleFailedDelete() {
-      // remove the spill file from disk
-      File file = spillWriters.removeFirst().getFile();
-      if (file != null && file.exists() && !file.delete()) {
-        logger.error("Was unable to delete spill file {}", 
file.getAbsolutePath());
+      if (spillWriters.size() > 0) {
+        // remove the spill file from disk
+        File file = spillWriters.removeFirst().getFile();
+        if (file != null && file.exists() && !file.delete()) {
+          logger.error("Was unable to delete spill file {}", 
file.getAbsolutePath());
+        }
       }
     }
   }
diff --git 
a/core/src/test/java/org/apache/spark/unsafe/map/AbstractBytesToBytesMapSuite.java
 
b/core/src/test/java/org/apache/spark/unsafe/map/AbstractBytesToBytesMapSuite.java
index 7741654..da11fd6 100644
--- 
a/core/src/test/java/org/apache/spark/unsafe/map/AbstractBytesToBytesMapSuite.java
+++ 
b/core/src/test/java/org/apache/spark/unsafe/map/AbstractBytesToBytesMapSuite.java
@@ -560,6 +560,8 @@ public abstract class AbstractBytesToBytesMapSuite {
         iter2.next();
       }
       assertFalse(iter2.hasNext());
+      // calls hasNext twice deliberately, make sure it's idempotent
+      assertFalse(iter2.hasNext());
     } finally {
       map.free();
       for (File spillFile : spillFilesCreated) {


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org
For additional commands, e-mail: commits-h...@spark.apache.org