Luo Chen has posted comments on this change.

Change subject: [NO ISSUE][STO][IDX] LSM storage cleanup
......................................................................


Patch Set 16:

(3 comments)

A few minor comments...

https://asterix-gerrit.ics.uci.edu/#/c/2014/16/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/api/ILSMWithBuddyDiskComponent.java
File 
hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/api/ILSMWithBuddyDiskComponent.java:

PS16, Line 81: createIndexWithBuddyBulkLoader
What is the purpose of this method since we already have 
createComponentBulkLoader?


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

PS16, Line 26: IChainedComponentBulkLoader
Do we need such a bulk loader with special name "chained" here? Based on its 
interfaces, it's almost identical to normal component bulk loaders. The only 
difference is that its implementation would handle the task of chaining 
multiple bulk loaders together, which should not be visible to callers.


https://asterix-gerrit.ics.uci.edu/#/c/2014/16/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/api/IInPlaceInvertedIndex.java
File 
hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/api/IInPlaceInvertedIndex.java:

PS16, Line 52: 
It seems this method is lifted to IIndex. Any motivation to do so?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6739d751b990e7a28e03e32a5de6e2b670d37a1e
Gerrit-PatchSet: 16
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Ildar Absalyamov <[email protected]>
Gerrit-Reviewer: Ian Maxon <[email protected]>
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-Reviewer: Luo Chen <[email protected]>
Gerrit-Reviewer: abdullah alamoudi <[email protected]>
Gerrit-HasComments: Yes

Reply via email to