Yingyi Bu has posted comments on this change.

Change subject: Add Disk Component Scan operation for primary LSM index
......................................................................


Patch Set 4:

(6 comments)

https://asterix-gerrit.ics.uci.edu/#/c/1791/4/hyracks-fullstack/hyracks/hyracks-api/src/main/resources/errormsg/en.properties
File 
hyracks-fullstack/hyracks/hyracks-api/src/main/resources/errormsg/en.properties:

PS4, Line 77: Secondary
"supported in Secondary LSM Index"-> "allowed for a secondary index" ?


https://asterix-gerrit.ics.uci.edu/#/c/1791/4/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/LSMBTreeDiskComponentScanCursor.java
File 
hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/LSMBTreeDiskComponentScanCursor.java:

PS4, Line 100:  // re-use
There are a few null-handling which tries to re-use objects. But I wonder 
whether/when open() can be call multiple times for a scan?   

It looks those non-null code branches are not covered by 
LSMBTreeScanDiskComponentsTest.

If it is never the case, maybe we can simplify the code?


PS4, Line 174: warning
throw an exception?


PS4, Line 239: elementA
return elementA.getCursorIndex() > elementB.getCursorIndex()? -1: 1;


https://asterix-gerrit.ics.uci.edu/#/c/1791/4/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/LSMHarness.java
File 
hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/LSMHarness.java:

PS4, Line 465: ctx.getSearchOperationCallback().before(null);
So disk component scan doesn't do any locking.   I remember that you said that 
you saw that inserts/upserts are still possible during CREATE INDEX, but 
Abdullah said that should be bug.


Can you still see it on the latest master?


If it still happens on the latest master?  Do we have a JIRA issue to track 
that?


PS4, Line 479: HyracksDataException
HyracksDataException.create(e);


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1791
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I31b2c67c58cb0a440c1d2c26400af322e2f1c1e5
Gerrit-PatchSet: 4
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Luo Chen <cl...@uci.edu>
Gerrit-Reviewer: Ian Maxon <ima...@apache.org>
Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Jianfeng Jia <jianfeng....@gmail.com>
Gerrit-Reviewer: Luo Chen <cl...@uci.edu>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-Reviewer: Yingyi Bu <buyin...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <bamou...@gmail.com>
Gerrit-HasComments: Yes

Reply via email to