Yingyi Bu has posted comments on this change. Change subject: Index-Only Plan Patch Set 4: Index SearchCursor adjustment ......................................................................
Patch Set 9: (6 comments) https://asterix-gerrit.ics.uci.edu/#/c/1630/8/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/LSMBTreePointSearchCursor.java File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/LSMBTreePointSearchCursor.java: PS8, Line 109: ; Should complete and cancel be placed as the first line in "finally" blocks such that we don't leak locks if any exception is thrown after lock acquisition but before unlocking? https://asterix-gerrit.ics.uci.edu/#/c/1630/9/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/LSMBTreePointSearchCursor.java File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/LSMBTreePointSearchCursor.java: PS9, Line 83: searchCallback should complete() always be called at the first line of a finally block so that in case anything fails since searchCallback.reconcile(...) can not lead to leaked locks? E.g., what if the query gets cancelled? PS9, Line 109: true should complete() always be called at the first line of a finally block so that in case anything fails since searchCallback.proceed(...) can not lead to leaked locks? E.g., what if the query gets cancelled? PS9, Line 114: searchCallback should cancel always be called at the first line of a finally block so that in case anything fails since searchCallback.reconcile(...) can not lead to leaked locks? E.g., what if the query gets cancelled? https://asterix-gerrit.ics.uci.edu/#/c/1630/9/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/LSMBTreeRangeSearchCursor.java File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/LSMBTreeRangeSearchCursor.java: PS9, Line 131: . Is it a no-op, essentially? you call lock and then unlock()? https://asterix-gerrit.ics.uci.edu/#/c/1630/9/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-rtree/src/main/java/org/apache/hyracks/storage/am/lsm/rtree/impls/LSMRTreeWithAntiMatterTuplesSearchCursor.java File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-rtree/src/main/java/org/apache/hyracks/storage/am/lsm/rtree/impls/LSMRTreeWithAntiMatterTuplesSearchCursor.java: PS9, Line 191: cancelProceed We didn't add cancel proceed. -- To view, visit https://asterix-gerrit.ics.uci.edu/1630 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I299b1858b7875ffc116f8f3115d319fe7b53a537 Gerrit-PatchSet: 9 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Taewoo Kim <wangs...@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-HasComments: Yes