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

[sr...@apache.org]

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

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


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

commit 5124558eb6ce561978231e3d9db9b86878e63ca9
Author: Xianjin YE <ad...@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 <ad...@gmail.com>
    Signed-off-by: Sean Owen <sr...@gmail.com>
    (cherry picked from commit 13391683e7a863671d3d719dc81e20ec2a870725)
    Signed-off-by: Sean Owen <sr...@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 d7940fc..f474c30 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
@@ -393,10 +393,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 f4e952f..f35176a 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
@@ -576,6 +576,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-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org