abdullah alamoudi has posted comments on this change. Change subject: Cleanup and bug fixes in Feeds pipeline ......................................................................
Patch Set 12: (20 comments) https://asterix-gerrit.ics.uci.edu/#/c/1523/12/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/exceptions/ExceptionUtils.java File asterixdb/asterix-common/src/main/java/org/apache/asterix/common/exceptions/ExceptionUtils.java: PS12, Line 40: HyracksDataException > return HyracksDataException.create(th); Done https://asterix-gerrit.ics.uci.edu/#/c/1523/12/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/adapter/factory/GenericAdapterFactory.java File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/adapter/factory/GenericAdapterFactory.java: PS12, Line 186: /** > Make the comments more complete? E.g., used in what kind of scenarios in e Done https://asterix-gerrit.ics.uci.edu/#/c/1523/12/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/dataflow/FeedRecordDataFlowController.java File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/dataflow/FeedRecordDataFlowController.java: PS12, Line 64: synchronized (this) { : wait(INTERVAL); : } > Does anyone notify "this"? If not, this should be replaced with Thread.sle No one does. Done. Chances are we will remove it once we revisit the reader interface at some point https://asterix-gerrit.ics.uci.edu/#/c/1523/12/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/declared/MetadataProvider.java File asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/declared/MetadataProvider.java: PS12, Line 365: this > What does this comment mean? Done https://asterix-gerrit.ics.uci.edu/#/c/1523/12/asterixdb/asterix-transactions/src/main/java/org/apache/asterix/transaction/management/runtime/CommitRuntime.java File asterixdb/asterix-transactions/src/main/java/org/apache/asterix/transaction/management/runtime/CommitRuntime.java: PS12, Line 114: this > open an issue? Done. https://issues.apache.org/jira/browse/ASTERIXDB-1824 https://asterix-gerrit.ics.uci.edu/#/c/1523/12/hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/api/AbstractPointable.java File hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/api/AbstractPointable.java: PS12, Line 33: return > comment on the parameter and return. Done PS12, Line 35: copy > Can we rename to copyTo/Into for clarity? Done PS12, Line 36: arraycopy > boundary/length check of copy? the System.arraycopy will do the check PS12, Line 36: arraycopy > I guess arraycopy will throw ArrayIndexOutOfBoundsException, should we just Done https://asterix-gerrit.ics.uci.edu/#/c/1523/12/hyracks-fullstack/hyracks/hyracks-dataflow-common/src/main/java/org/apache/hyracks/dataflow/common/io/MessagingFrameTupleAppender.java File hyracks-fullstack/hyracks/hyracks-dataflow-common/src/main/java/org/apache/hyracks/dataflow/common/io/MessagingFrameTupleAppender.java: PS12, Line 46: protected > Can we implement ProgressFrameTupleAppender as a separate FrameTupleAppende Done https://asterix-gerrit.ics.uci.edu/#/c/1523/12/hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/connectors/PartitionDataWriter.java File hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/connectors/PartitionDataWriter.java: PS12, Line 42: protected > Can we implement ProgressPartitionDataWriter as a separate IFrameWriter, ra Done PS12, Line 134: failed = true; > do we need a new flag? can we just use isOpen for this? we need the failed flag for the close case PS12, Line 156: if (!failed) { > should we flush if not opened? can we eliminate failed flag and just use i the fail flag has to be there because in the close call, we need to know whether there was a failure in which case, we shouldn't flush the remaining tuples https://asterix-gerrit.ics.uci.edu/#/c/1523/12/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/api/IFrameOperationCallback.java File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/api/IFrameOperationCallback.java: PS12, Line 23: FunctionalInterface > Comment the interface, i.e., why do we need the interface, what does it do, Done PS12, Line 25: frameCompleted > comment the interface method. Done https://asterix-gerrit.ics.uci.edu/#/c/1523/12/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/api/IFrameOperationCallbackFactory.java File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/api/IFrameOperationCallbackFactory.java: PS12, Line 26: IFrameOperationCallbackFactory > comment the interface and its method. Done https://asterix-gerrit.ics.uci.edu/#/c/1523/12/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/api/ILSMHarness.java File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/api/ILSMHarness.java: PS12, Line 63: updateMeta > java doc for the two interface methods. Done https://asterix-gerrit.ics.uci.edu/#/c/1523/12/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/api/ILSMIndexAccessor.java File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/api/ILSMIndexAccessor.java: PS12, Line 142: ( > javadoc for the added methods. Done https://asterix-gerrit.ics.uci.edu/#/c/1523/12/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/LSMTreeIndexAccessor.java File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/LSMTreeIndexAccessor.java: PS12, Line 171: memory > open an issue? Done https://issues.apache.org/jira/browse/ASTERIXDB-1826 PS12, Line 179: DELETE > open an issue? Done -- To view, visit https://asterix-gerrit.ics.uci.edu/1523 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie97b2133ebecb7380cf0ba336e60ed714d06f8ee Gerrit-PatchSet: 12 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: abdullah alamoudi <bamou...@gmail.com> Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Gerrit-Reviewer: Michael Blow <mb...@apache.org> Gerrit-Reviewer: Yingyi Bu <buyin...@gmail.com> Gerrit-Reviewer: abdullah alamoudi <bamou...@gmail.com> Gerrit-HasComments: Yes
