abdullah alamoudi has posted comments on this change. Change subject: Fix upsert deadlock and upsert with filtered primary only ......................................................................
Patch Set 3: (9 comments) https://asterix-gerrit.ics.uci.edu/#/c/1762/2/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/ITreeIndexCursor.java File hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/ITreeIndexCursor.java: PS2, Line 32: isExclusiveLatchNod > Rename this to isExclusiveLatchNodes while you are at it. Done https://asterix-gerrit.ics.uci.edu/#/c/1762/2/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/tuples/PermutingTupleReference.java File hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/tuples/PermutingTupleReference.java: PS2, Line 37: > Is there a need to add this method? Done https://asterix-gerrit.ics.uci.edu/#/c/1762/2/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/ExternalBTree.java File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/ExternalBTree.java: PS2, Line 264: createDisk > Suggestion: It might better to override getLsmHarness here and return Exter Done https://asterix-gerrit.ics.uci.edu/#/c/1762/2/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/ExternalBTreeWithBuddy.java File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/ExternalBTreeWithBuddy.java: PS2, Line 245: ew > Can this be replaced by a static factory similar to ExternalBTree? not as straight forward since buddyBtree is different per index. we could probably get the buddyBtreeFields from the ctx but would rather do that in a separate change. https://asterix-gerrit.ics.uci.edu/#/c/1762/2/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/LSMBTreeBulkLoader.java File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/LSMBTreeBulkLoader.java: Line 101: private void cleanupArtifacts() throws HyracksDataException { > Make it private Done https://asterix-gerrit.ics.uci.edu/#/c/1762/2/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/AbstractLSMIndex.java File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/AbstractLSMIndex.java: PS2, Line 88: MComponentF > I have always wanted to rename this thing. Can we rename it to something li Done PS2, Line 516: kComponents.add(diskCompone > I think this needs to be synchronized as well not needed there is a synchronization on the allocation call which will do another check before allocating. This gets called frequently so adding a non-needed additional synchronization would be bad. Line 625: for (ILSMMemoryComponent c : memoryComponents) { > hb Done https://asterix-gerrit.ics.uci.edu/#/c/1762/2/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-rtree/src/main/java/org/apache/hyracks/storage/am/lsm/rtree/impls/ExternalRTree.java File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-rtree/src/main/java/org/apache/hyracks/storage/am/lsm/rtree/impls/ExternalRTree.java: PS2, Line 208: omonentFileRe > Same suggestion regarding overriding getLsmHarness Done -- To view, visit https://asterix-gerrit.ics.uci.edu/1762 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I93fac0f27ab0b3cc071ff38aef90d850cbbce488 Gerrit-PatchSet: 3 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: abdullah alamoudi <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Murtadha Hubail <[email protected]> Gerrit-Reviewer: abdullah alamoudi <[email protected]> Gerrit-HasComments: Yes
