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

mhubail pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/asterixdb.git

commit 225f47c44434ff7a178caf6c95131482e39cac26
Author: Ali Alsuliman <[email protected]>
AuthorDate: Mon Jul 29 19:03:23 2024 -0700

    [ASTERIXDB-3472][HYR] Catch Task exceptions
    
    - user model changes: no
    - storage format changes: no
    - interface changes: no
    
    Details:
    When executing a Task, catch any exception from the different try blocks
    to prevent the loss of the original exception.
    
    This patch includes cherry-picks from:
    https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18317
    
    Ext-ref: MB-62949
    Change-Id: I5e0f1e6b906c15932a3bb7ae740b5a7c89198cae
    Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18565
    Integration-Tests: Jenkins <[email protected]>
    Tested-by: Jenkins <[email protected]>
    Reviewed-by: Ali Alsuliman <[email protected]>
    Reviewed-by: Michael Blow <[email protected]>
---
 .../org/apache/hyracks/api/comm/IFrameReader.java  |  2 +-
 .../org/apache/hyracks/api/comm/IFrameWriter.java  |  2 +-
 .../hyracks/api/comm/IPartitionCollector.java      |  2 +-
 .../org/apache/hyracks/api/util/CleanupUtils.java  | 49 +++++++++++++++++++---
 .../java/org/apache/hyracks/control/nc/Task.java   | 23 +++++-----
 5 files changed, 59 insertions(+), 19 deletions(-)

diff --git 
a/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/comm/IFrameReader.java
 
b/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/comm/IFrameReader.java
index 62eacc8b27..3e7e2aba9f 100644
--- 
a/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/comm/IFrameReader.java
+++ 
b/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/comm/IFrameReader.java
@@ -20,7 +20,7 @@ package org.apache.hyracks.api.comm;
 
 import org.apache.hyracks.api.exceptions.HyracksDataException;
 
-public interface IFrameReader {
+public interface IFrameReader extends AutoCloseable {
     void open() throws HyracksDataException;
 
     boolean nextFrame(IFrame frame) throws HyracksDataException;
diff --git 
a/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/comm/IFrameWriter.java
 
b/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/comm/IFrameWriter.java
index 19d7afb2a6..a49f2267cf 100644
--- 
a/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/comm/IFrameWriter.java
+++ 
b/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/comm/IFrameWriter.java
@@ -56,7 +56,7 @@ import org.apache.hyracks.api.exceptions.HyracksDataException;
  * Note: If the call to {@link IFrameWriter#open()} failed, the {@link 
IFrameWriter#close()} must still be called by the
  * producer.
  */
-public interface IFrameWriter {
+public interface IFrameWriter extends AutoCloseable {
     /**
      * First call to allocate any resources.
      */
diff --git 
a/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/comm/IPartitionCollector.java
 
b/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/comm/IPartitionCollector.java
index 4a977ec404..7b528758d8 100644
--- 
a/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/comm/IPartitionCollector.java
+++ 
b/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/comm/IPartitionCollector.java
@@ -25,7 +25,7 @@ import org.apache.hyracks.api.exceptions.HyracksException;
 import org.apache.hyracks.api.job.JobId;
 import org.apache.hyracks.api.partitions.PartitionId;
 
-public interface IPartitionCollector {
+public interface IPartitionCollector extends AutoCloseable {
     public JobId getJobId();
 
     public ConnectorDescriptorId getConnectorId();
diff --git 
a/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/util/CleanupUtils.java
 
b/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/util/CleanupUtils.java
index a29a04aee6..5ebfba428b 100644
--- 
a/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/util/CleanupUtils.java
+++ 
b/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/util/CleanupUtils.java
@@ -102,26 +102,63 @@ public class CleanupUtils {
         }
     }
 
+    public static Throwable close(AutoCloseable[] closables, Throwable root) {
+        return close(closables, root, false);
+    }
+
+    public static Throwable closeSilently(AutoCloseable[] closables, Throwable 
root) {
+        return close(closables, root, true);
+    }
+
     /**
      * Close the AutoCloseable and suppress any Throwable thrown by the close 
call.
      * This method must NEVER throw any Throwable
      *
-     * @param closable
+     * @param closables
      *            the resource to close
      * @param root
      *            the first exception encountered during release of resources
      * @return the root Throwable if not null or a new Throwable if any was 
thrown, otherwise, it returns null
      */
+    private static Throwable close(AutoCloseable[] closables, Throwable root, 
boolean silent) {
+        if (closables != null) {
+            for (AutoCloseable closable : closables) {
+                root = close(closable, root, silent);
+            }
+        }
+        return root;
+    }
+
     public static Throwable close(AutoCloseable closable, Throwable root) {
+        return close(closable, root, false);
+    }
+
+    public static Throwable closeSilently(AutoCloseable closable, Throwable 
root) {
+        return close(closable, root, true);
+    }
+
+    /**
+     * Close the AutoCloseable and suppress any Throwable thrown by the close 
call.
+     * This method must NEVER throw any Throwable
+     *
+     * @param closable
+     *            the resource to close
+     * @param root
+     *            the first exception encountered during release of resources
+     * @return the root Throwable if not null or a new Throwable if any was 
thrown, otherwise, it returns null
+     */
+    private static Throwable close(AutoCloseable closable, Throwable root, 
boolean silent) {
         if (closable != null) {
             try {
                 closable.close();
             } catch (Throwable th) { // NOSONAR Will be suppressed
-                try {
-                    LOGGER.log(ExceptionUtils.causedByInterrupt(th) ? 
Level.DEBUG : Level.WARN,
-                            "Failure closing a closeable resource {}", 
closable.getClass().getName(), th);
-                } catch (Throwable loggingFailure) { // NOSONAR: Ignore 
catching Throwable
-                    // NOSONAR ignore logging failure
+                if (!silent) {
+                    try {
+                        LOGGER.log(ExceptionUtils.causedByInterrupt(th) ? 
Level.DEBUG : Level.WARN,
+                                "Failure closing a closeable resource {}", 
closable.getClass().getName(), th);
+                    } catch (Throwable loggingFailure) { // NOSONAR: Ignore 
catching Throwable
+                        // NOSONAR ignore logging failure
+                    }
                 }
                 root = ExceptionUtils.suppress(root, th); // NOSONAR
             }
diff --git 
a/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-nc/src/main/java/org/apache/hyracks/control/nc/Task.java
 
b/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-nc/src/main/java/org/apache/hyracks/control/nc/Task.java
index 0c5c23329f..0bf74ee1cb 100644
--- 
a/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-nc/src/main/java/org/apache/hyracks/control/nc/Task.java
+++ 
b/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-nc/src/main/java/org/apache/hyracks/control/nc/Task.java
@@ -63,6 +63,7 @@ import 
org.apache.hyracks.api.job.profiling.counters.ICounterContext;
 import org.apache.hyracks.api.partitions.PartitionId;
 import org.apache.hyracks.api.resources.IDeallocatable;
 import org.apache.hyracks.api.result.IResultPartitionManager;
+import org.apache.hyracks.api.util.CleanupUtils;
 import org.apache.hyracks.api.util.ExceptionUtils;
 import org.apache.hyracks.api.util.JavaSerializationUtils;
 import org.apache.hyracks.control.common.job.PartitionState;
@@ -406,6 +407,7 @@ public class Task implements IHyracksTaskContext, 
ICounterContext, Runnable {
         if (aborted) {
             return;
         }
+        Throwable originalEx = null;
         try {
             collector.open();
             try {
@@ -430,23 +432,24 @@ public class Task implements IHyracksTaskContext, 
ICounterContext, Runnable {
                             buffer.compact();
                         }
                     } catch (Exception e) {
-                        try {
-                            writer.fail();
-                        } catch (HyracksDataException e1) {
-                            e.addSuppressed(e1);
-                        }
-                        throw e;
+                        originalEx = e;
+                        CleanupUtils.fail(writer, originalEx);
                     } finally {
-                        writer.close();
+                        originalEx = CleanupUtils.closeSilently(writer, 
originalEx);
                     }
                 } finally {
-                    reader.close();
+                    originalEx = CleanupUtils.closeSilently(reader, 
originalEx);
                 }
+            } catch (Exception e) {
+                originalEx = ExceptionUtils.suppress(originalEx, e);
             } finally {
-                collector.close();
+                originalEx = CleanupUtils.closeSilently(collector, originalEx);
             }
         } catch (Exception e) {
-            throw HyracksDataException.create(e);
+            originalEx = ExceptionUtils.suppress(originalEx, e);
+        }
+        if (originalEx != null) {
+            throw HyracksDataException.create(originalEx);
         }
     }
 

Reply via email to