>From Ali Alsuliman <[email protected]>: Ali Alsuliman has submitted this change. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18707 )
Change subject: [ASTERIXDB-3472][HYR] Use CleanupUtils.fail/close in ProbeAndJoinActivityNode ...................................................................... [ASTERIXDB-3472][HYR] Use CleanupUtils.fail/close in ProbeAndJoinActivityNode - user model changes: no - storage format changes: no - interface changes: no Details: In the close() of the hash-join ProbeAndJoinActivityNode, use CleanupUtils.fail()/close() when closing the next writer. The reason is if all the build partitions are spilled, pushing the frames from the ProbeAndJoinActivityNode to the next writer will happen in the close(), not nextFrame(). In such cases, any exception that happens in the next writer is caught in ProbeAndJoinActivityNode.close(). After that, it calls fail()/close() on the next writer. Those fail()/close() should be done using CleanupUtils so that the original caught exception is not lost. Ext-ref: MB-62949 Change-Id: I1cdee5ed457b72b4f6b3731fba3542898dec9bf3 Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18707 Tested-by: Jenkins <[email protected]> Reviewed-by: Michael Blow <[email protected]> --- M hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/join/OptimizedHybridHashJoin.java M hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/join/OptimizedHybridHashJoinOperatorDescriptor.java 2 files changed, 40 insertions(+), 5 deletions(-) Approvals: Michael Blow: Looks good to me, approved Jenkins: Verified diff --git a/hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/join/OptimizedHybridHashJoin.java b/hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/join/OptimizedHybridHashJoin.java index 04f5fe8..ddf94db 100644 --- a/hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/join/OptimizedHybridHashJoin.java +++ b/hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/join/OptimizedHybridHashJoin.java @@ -255,7 +255,7 @@ } } - public void fail() throws HyracksDataException { + public void fail() { for (RunFileWriter writer : buildRFWriters) { if (writer != null) { CleanupUtils.fail(writer, null); diff --git a/hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/join/OptimizedHybridHashJoinOperatorDescriptor.java b/hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/join/OptimizedHybridHashJoinOperatorDescriptor.java index ed53a7e..91e94dc 100644 --- a/hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/join/OptimizedHybridHashJoinOperatorDescriptor.java +++ b/hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/join/OptimizedHybridHashJoinOperatorDescriptor.java @@ -47,6 +47,7 @@ import org.apache.hyracks.api.job.JobId; import org.apache.hyracks.api.job.profiling.IOperatorStats; import org.apache.hyracks.api.job.profiling.NoOpOperatorStats; +import org.apache.hyracks.api.util.CleanupUtils; import org.apache.hyracks.dataflow.common.comm.io.ArrayTupleBuilder; import org.apache.hyracks.dataflow.common.comm.io.FrameTupleAccessor; import org.apache.hyracks.dataflow.common.comm.io.FrameTupleAppender; @@ -440,6 +441,7 @@ } return; } + Throwable ex = null; try { try { state.hybridHJ.completeProbe(writer); @@ -470,19 +472,21 @@ joinPartitionPair(bReader, pReader, bSize, pSize, 1); } } catch (Exception e) { + ex = e; if (state.hybridHJ != null) { state.hybridHJ.fail(); } // Since writer.nextFrame() is called in the above "try" body, we have to call writer.fail() // to send the failure signal to the downstream, when there is a throwable thrown. - writer.fail(); + CleanupUtils.fail(writer, ex); // Clear temp files as this.fail() nor this.close() will no longer be called after close(). state.hybridHJ.clearBuildTempFiles(); state.hybridHJ.clearProbeTempFiles(); - // Re-throw the whatever is caught. - throw e; } finally { - writer.close(); + ex = CleanupUtils.close(writer, ex); + } + if (ex != null) { + throw HyracksDataException.create(ex); } } -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18707 To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-Project: asterixdb Gerrit-Branch: stabilization-6a10f3f81d Gerrit-Change-Id: I1cdee5ed457b72b4f6b3731fba3542898dec9bf3 Gerrit-Change-Number: 18707 Gerrit-PatchSet: 2 Gerrit-Owner: Ali Alsuliman <[email protected]> Gerrit-Reviewer: Ali Alsuliman <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Michael Blow <[email protected]> Gerrit-Reviewer: Murtadha Hubail <[email protected]> Gerrit-Reviewer: Peeyush Gupta <[email protected]> Gerrit-MessageType: merged
