Michael Blow has posted comments on this change. Change subject: Add Test NodeController and Test Data Generator ......................................................................
Patch Set 7: (10 comments) https://asterix-gerrit.ics.uci.edu/#/c/962/7//COMMIT_MSG Commit Message: Line 7: Add Test NodeController and Test Data Generator Please update the commit message to reflect the updated scope of the change. https://asterix-gerrit.ics.uci.edu/#/c/962/7/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/utils/FrameStack.java File asterixdb/asterix-common/src/main/java/org/apache/asterix/common/utils/FrameStack.java: Line 87: ByteBuffer aBuffer = emptyBuffers.isEmpty() ? ByteBuffer.allocate(frameSize) : emptyBuffers.pop(); If you change to poll(), you can avoid two calls to emptyBuffers in case of free buffer available. https://asterix-gerrit.ics.uci.edu/#/c/962/7/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/utils/StoragePathUtil.java File asterixdb/asterix-common/src/main/java/org/apache/asterix/common/utils/StoragePathUtil.java: Line 83: file.getParentFile().mkdirs(); Note, this can fail in case two threads try to create the dir at same time. https://asterix-gerrit.ics.uci.edu/#/c/962/7/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: Line 63: aString.append("Snapshot, "); > Should we align this with the case label? I don't understand this. https://asterix-gerrit.ics.uci.edu/#/c/962/7/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/freepage/LinkedMetaDataPageManager.java File hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/freepage/LinkedMetaDataPageManager.java: Line 507: if (!appendOnly || (appendOnly && confiscatedMetaNode == null)) { > BLOCKER SonarQube violation: Well, this SonarQube observation is pretty broken. Line 519: if (!appendOnly || (appendOnly && confiscatedMetaNode == null)) { > BLOCKER SonarQube violation: Well, this SonarQube observation is pretty broken. https://asterix-gerrit.ics.uci.edu/#/c/962/7/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/AbstractLSMComponent.java File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/AbstractLSMComponent.java: Line 42: this(null, -1L); Can this call this(null) as before, to avoid the -1L in two places? https://asterix-gerrit.ics.uci.edu/#/c/962/7/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/AbstractMemoryLSMComponent.java File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/AbstractMemoryLSMComponent.java: Line 35: public AbstractMemoryLSMComponent(IVirtualBufferCache vbc, boolean isActive, ILSMComponentFilter filter, Can we keep the old ctor around, then we can eliminate changes to subclasses which are just passing in the -1L. https://asterix-gerrit.ics.uci.edu/#/c/962/7/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/impls/LSMInvertedIndexMemoryComponent.java File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/impls/LSMInvertedIndexMemoryComponent.java: Line 36: super(vbc, isActive, filter, -1L); Can we keep the old super ctor around, then we can eliminate this change. https://asterix-gerrit.ics.uci.edu/#/c/962/7/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-rtree/src/main/java/org/apache/hyracks/storage/am/lsm/rtree/impls/LSMRTreeMemoryComponent.java File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-rtree/src/main/java/org/apache/hyracks/storage/am/lsm/rtree/impls/LSMRTreeMemoryComponent.java: Line 36: super(vbc, isActive, filter, -1L); Can we keep the old super ctor around, then we can eliminate this change. -- To view, visit https://asterix-gerrit.ics.uci.edu/962 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3b9aa8de758b7d26ca34868b16e5ce693e0c0243 Gerrit-PatchSet: 7 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: Till Westmann <[email protected]> Gerrit-Reviewer: abdullah alamoudi <[email protected]> Gerrit-HasComments: Yes
