Yingyi Bu has posted comments on this change.

Change subject: Implemented LSM disk components alignment
......................................................................


Patch Set 18:

(3 comments)

https://asterix-gerrit.ics.uci.edu/#/c/1725/18/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/CorrelatedPrefixMergePolicy.java
File 
asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/CorrelatedPrefixMergePolicy.java:

PS18, Line 131: isCorrelated
I wonder whether the CorrelatedMergePolicy can stick to the original 
ILSMMergePolicy interface.


It seems to me that the method isCorrelated shouldn't be part of the 
ILSMMergePolicy interface. In principle, an interface shouldn't be aware of a 
certain implementation and the polymorphism should be achieved through 
dynamically dispatching instead of statically dispatching by calling 
isCorrelated().   


As a result of adding isCorrelated() to the interface, all callers of the merge 
policy become aware of the correlated merge policy.


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

PS18, Line 53: setOperationGroup
I wonder is it mandatory that each individual operation needs to be aware of 
their belonging group?  It feels only a group needs to know all its operations 
but not the other way around?


https://asterix-gerrit.ics.uci.edu/#/c/1725/18/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:

PS18, Line 513: setNewComponent
Why the old contract doesn't work now?
operation.getCallback().afterOperation(...)

lsmIndex.markAsValid(...) has been done inside lsmIndex.flush()?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I64bf34e255def72adc73b9f87cfa628a172ea694
Gerrit-PatchSet: 18
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Luo Chen <[email protected]>
Gerrit-Reviewer: Ian Maxon <[email protected]>
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-Reviewer: Jianfeng Jia <[email protected]>
Gerrit-Reviewer: Luo Chen <[email protected]>
Gerrit-Reviewer: Till Westmann <[email protected]>
Gerrit-Reviewer: Yingyi Bu <[email protected]>
Gerrit-Reviewer: abdullah alamoudi <[email protected]>
Gerrit-HasComments: Yes

Reply via email to