Yingyi Bu has posted comments on this change. Change subject: Add flush() to IFrameWriter ......................................................................
Patch Set 5: (19 comments) I think in those places where flush() shouldn't be called, we should explicitly throw exceptions. Sort, join, and group-by operators are such examples. https://asterix-gerrit.ics.uci.edu/#/c/584/5/algebricks/algebricks-runtime/src/main/java/org/apache/hyracks/algebricks/runtime/operators/std/StreamProjectRuntimeFactory.java File algebricks/algebricks-runtime/src/main/java/org/apache/hyracks/algebricks/runtime/operators/std/StreamProjectRuntimeFactory.java: Line 90: writer.flush(); appender.flush()? https://asterix-gerrit.ics.uci.edu/#/c/584/5/algebricks/algebricks-runtime/src/main/java/org/apache/hyracks/algebricks/runtime/operators/std/StreamSelectRuntimeFactory.java File algebricks/algebricks-runtime/src/main/java/org/apache/hyracks/algebricks/runtime/operators/std/StreamSelectRuntimeFactory.java: Line 164: writer.flush(); appender.flush()? https://asterix-gerrit.ics.uci.edu/#/c/584/5/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/comm/IFrameAppender.java File hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/comm/IFrameAppender.java: Line 51: * Flush the frame content to the given writer. Flush-->Write Line 52: * Clear the inner buffer after flush if {@code clear} is <code>true</code>. flush-->write Line 65: public default void flush(IFrameWriter writer) throws HyracksDataException { do you need the clear bit parameter here? Or, if you think the parameter is not necessary, it would be good to add to the comments that the frame is always cleared. https://asterix-gerrit.ics.uci.edu/#/c/584/5/hyracks/hyracks-dataflow-common/src/main/java/org/apache/hyracks/dataflow/common/comm/io/FrameTupleAppenderWrapper.java File hyracks/hyracks-dataflow-common/src/main/java/org/apache/hyracks/dataflow/common/comm/io/FrameTupleAppenderWrapper.java: Line 49: public void flush() throws HyracksDataException { The meaning of this flush() call meant "write". Thus, rename flush() to write() and add a new flush(). https://asterix-gerrit.ics.uci.edu/#/c/584/5/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/group/preclustered/PreclusteredGroupWriter.java File hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/group/preclustered/PreclusteredGroupWriter.java: Line 190: // operator should only send its output once all of its input has been consumed. hence, this is a no op throw an exception here if this method should never be called. https://asterix-gerrit.ics.uci.edu/#/c/584/5/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/join/HybridHashJoinOperatorDescriptor.java File hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/join/HybridHashJoinOperatorDescriptor.java: Line 577: // flush() on any join operator is a no op throw an exception here https://asterix-gerrit.ics.uci.edu/#/c/584/5/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/join/InMemoryHashJoinOperatorDescriptor.java File hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/join/InMemoryHashJoinOperatorDescriptor.java: Line 257: // flush() on any join operator is a no op throw an exception here. https://asterix-gerrit.ics.uci.edu/#/c/584/5/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/join/NestedLoopJoinOperatorDescriptor.java File hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/join/NestedLoopJoinOperatorDescriptor.java: Line 222: throw an exception here. https://asterix-gerrit.ics.uci.edu/#/c/584/5/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/join/OptimizedHybridHashJoinOperatorDescriptor.java File hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/join/OptimizedHybridHashJoinOperatorDescriptor.java: Line 126: // Flags added for test purpose It seems your format is different? I tried to format this format in my Eclipse and nothing gets changed... Line 680: // flush() on any join operator is a no op throw an exception here. https://asterix-gerrit.ics.uci.edu/#/c/584/5/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/misc/MaterializingOperatorDescriptor.java File hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/misc/MaterializingOperatorDescriptor.java: Line 121: } throw an exception? https://asterix-gerrit.ics.uci.edu/#/c/584/5/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/misc/PrinterOperatorDescriptor.java File hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/misc/PrinterOperatorDescriptor.java: Line 68: // flush() on data writer operators is a no op why data writer operator is a no op? It seems the results should be "flushed" out.. https://asterix-gerrit.ics.uci.edu/#/c/584/5/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/misc/SplitVectorOperatorDescriptor.java File hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/misc/SplitVectorOperatorDescriptor.java: Line 113: // flush() on data writer operators is a no op why this is a no op? https://asterix-gerrit.ics.uci.edu/#/c/584/5/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/sort/ExternalSortRunGenerator.java File hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/sort/ExternalSortRunGenerator.java: Line 120: // flush() on sort operators is a no op since the sort needs all of its input before sending any output throw an exception? https://asterix-gerrit.ics.uci.edu/#/c/584/5/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/sort/HeapSortRunGenerator.java File hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/sort/HeapSortRunGenerator.java: Line 104: // flush() on sort operators is a no op since the sort needs all of its input before sending any output throw an exception? https://asterix-gerrit.ics.uci.edu/#/c/584/5/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexSearchOperatorNodePushable.java File hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexSearchOperatorNodePushable.java: Line 193: writer.flush(); appender.flush()? https://asterix-gerrit.ics.uci.edu/#/c/584/5/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/dataflow/BinaryTokenizerOperatorNodePushable.java File hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/dataflow/BinaryTokenizerOperatorNodePushable.java: Line 172: } appender.flush()? -- To view, visit https://asterix-gerrit.ics.uci.edu/584 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I85424bab7965b71aac709280af066e1655457aa3 Gerrit-PatchSet: 5 Gerrit-Project: hyracks Gerrit-Branch: master Gerrit-Owner: abdullah alamoudi <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Murtadha Hubail <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-Reviewer: Yingyi Bu <[email protected]> Gerrit-Reviewer: abdullah alamoudi <[email protected]> Gerrit-HasComments: Yes
