Murtadha Hubail has posted comments on this change. Change subject: Add flush() to IFrameWriter ......................................................................
Patch Set 3: (8 comments) https://asterix-gerrit.ics.uci.edu/#/c/584/3/algebricks/algebricks-runtime/src/main/java/org/apache/hyracks/algebricks/runtime/operators/aggreg/AggregateRuntimeFactory.java File algebricks/algebricks-runtime/src/main/java/org/apache/hyracks/algebricks/runtime/operators/aggreg/AggregateRuntimeFactory.java: Line 147: public void flush() throws HyracksDataException { Add comment to why this operator doesn't need to implement flush() https://asterix-gerrit.ics.uci.edu/#/c/584/3/algebricks/algebricks-runtime/src/main/java/org/apache/hyracks/algebricks/runtime/operators/meta/AlgebricksMetaOperatorDescriptor.java File algebricks/algebricks-runtime/src/main/java/org/apache/hyracks/algebricks/runtime/operators/meta/AlgebricksMetaOperatorDescriptor.java: Line 163: if (startOfPipeline != null) { why would this be null? If the sequence of calls is done correctly, then this will never be null. https://asterix-gerrit.ics.uci.edu/#/c/584/3/hyracks/hyracks-control/hyracks-control-nc/src/main/java/org/apache/hyracks/control/nc/partitions/PipelinedPartition.java File hyracks/hyracks-control/hyracks-control-nc/src/main/java/org/apache/hyracks/control/nc/partitions/PipelinedPartition.java: Line 124: if (!failed) { better make the check before ensureConnected() https://asterix-gerrit.ics.uci.edu/#/c/584/3/hyracks/hyracks-dataflow-common/src/main/java/org/apache/hyracks/dataflow/common/comm/io/SerializingDataWriter.java File hyracks/hyracks-dataflow-common/src/main/java/org/apache/hyracks/dataflow/common/comm/io/SerializingDataWriter.java: Line 94: tupleAppender.flush(frameWriter, true); should this flush its frameWriter as well? https://asterix-gerrit.ics.uci.edu/#/c/584/3/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/connectors/LocalityAwarePartitionDataWriter.java File hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/connectors/LocalityAwarePartitionDataWriter.java: Line 157: for (IFrameWriter pWriter : pWriters) { I think you should flush the appenders first. https://asterix-gerrit.ics.uci.edu/#/c/584/3/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/connectors/PartitionDataWriter.java File hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/connectors/PartitionDataWriter.java: Line 147: int consumerPartitionCount = appenders.length; no need to assign this value. Line 151: pWriters[i].flush(); This should be outside the if statement https://asterix-gerrit.ics.uci.edu/#/c/584/3/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 170: appender.flush(writer, true); check if count > 0 -- 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: 3 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: Yingyi Bu <[email protected]> Gerrit-HasComments: Yes
