abdullah alamoudi has posted comments on this change. Change subject: Add flush() to IFrameWriter ......................................................................
Patch Set 3: (9 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() Done 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 th Done https://asterix-gerrit.ics.uci.edu/#/c/584/3/algebricks/algebricks-runtime/src/main/java/org/apache/hyracks/algebricks/runtime/operators/std/PartitioningSplitOperatorDescriptor.java File algebricks/algebricks-runtime/src/main/java/org/apache/hyracks/algebricks/runtime/operators/std/PartitioningSplitOperatorDescriptor.java: Line 93 Because this is in the close and so the buffer will not be used anymore. so why clear it! 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() Done 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? Done 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. Done 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. Done Line 151: pWriters[i].flush(); > This should be outside the if statement Done 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 I made them all consistent by not checking since a check is applied in the appender.flush method -- 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-Reviewer: abdullah alamoudi <[email protected]> Gerrit-HasComments: Yes
