abdullah alamoudi has posted comments on this change. Change subject: Ensure nextFrame and flush are not called in a failed pipeline ......................................................................
Patch Set 5: (10 comments) https://asterix-gerrit.ics.uci.edu/#/c/1618/5/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/dataflow/FeedRuntimeInputHandler.java File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/dataflow/FeedRuntimeInputHandler.java: PS5, Line 95: synchronized > why synchronized is needed? it is not needed as far as I know and we can safely remove the synchronized https://asterix-gerrit.ics.uci.edu/#/c/1618/5/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/dataflow/SyncFeedRuntimeInputHandler.java File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/feed/dataflow/SyncFeedRuntimeInputHandler.java: PS5, Line 54: unfail > Couldn't understand why you need to call writer.unfail() and it's only for You remember when we talked about how this interesting exception handling is done in the feeds outside the framewriter. the problem here is that exception has been handled outside the framewriter which has failed. if we remove the "bad" tuple and call next frame again, it will fail due to having the failed flag set to true. so we need to tell the writer that the failure has been handled outside the operator somehow. https://asterix-gerrit.ics.uci.edu/#/c/1618/5/hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/base/AbstractUnaryInputUnaryOutputOperatorNodePushable.java File hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/base/AbstractUnaryInputUnaryOutputOperatorNodePushable.java: PS5, Line 44: Override > The state machine enforcement is nice. But I think it should be moved to IF It is not special. I just thought it is a place to start with. PS5, Line 56: > Log the stack trace so that we know which operator does not do the right th Sure. PS5, Line 69: LOGGER > Log the stack trace so that we know which operator does not do the right th Sure. PS5, Line 79: unfail > This seems error-prone. Once an IFrameWriter failed, it should be failed f Feed is a special case :( where failure is handled outside the frame writer by removing the bad tuple. https://asterix-gerrit.ics.uci.edu/#/c/1618/5/hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/join/HybridHashJoinOperatorDescriptor.java File hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/join/HybridHashJoinOperatorDescriptor.java: PS5, Line 577: > The probe branch could be flushed since you might have sth. in the output b Done https://asterix-gerrit.ics.uci.edu/#/c/1618/5/hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/join/InMemoryHashJoinOperatorDescriptor.java File hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/join/InMemoryHashJoinOperatorDescriptor.java: PS5, Line 295: > The probe branch could be flushed since you might have sth. in the output b Done https://asterix-gerrit.ics.uci.edu/#/c/1618/5/hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/join/NestedLoopJoinOperatorDescriptor.java File hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/join/NestedLoopJoinOperatorDescriptor.java: PS5, Line 223: No > The probe branch could be flushed since you might have sth. in the output b Done https://asterix-gerrit.ics.uci.edu/#/c/1618/5/hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/join/OptimizedHybridHashJoinOperatorDescriptor.java File hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/join/OptimizedHybridHashJoinOperatorDescriptor.java: PS5, Line 820: } > The probe branch could be flushed since you might have sth. in the output b Done -- To view, visit https://asterix-gerrit.ics.uci.edu/1618 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9827b06f640858f27ec1bcca2a39991780bee3b1 Gerrit-PatchSet: 5 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: abdullah alamoudi <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Michael Blow <[email protected]> Gerrit-Reviewer: Yingyi Bu <[email protected]> Gerrit-Reviewer: abdullah alamoudi <[email protected]> Gerrit-HasComments: Yes
