Young-Seok Kim has posted comments on this change. Change subject: ASTERIXDB-1011: added flow control for merge policy See the design document here: https://cwiki.apache.org/confluence/display/ASTERIXDB/Flush-Operation+Flow+Control+For+Merge+Policy ......................................................................
Patch Set 3: (13 comments) https://asterix-gerrit.ics.uci.edu/#/c/795/3/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/ConstantMergePolicy.java File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/ConstantMergePolicy.java: Line 47: ILSMIndexAccessor accessor = (ILSMIndexAccessor) index.createAccessor(NoOpOperationCallback.INSTANCE, > Remove (ILSMIndexAccessor) cast Done Line 51: ILSMIndexAccessor accessor = (ILSMIndexAccessor) index.createAccessor(NoOpOperationCallback.INSTANCE, > Remove (ILSMIndexAccessor) cast Done Line 65: > remove white spaces, and use multiple line comment Done Line 108: ILSMIndexAccessor accessor = (ILSMIndexAccessor) index.createAccessor(NoOpOperationCallback.INSTANCE, > Remove (ILSMIndexAccessor) cast if you prefer to keep this code. Yes, I want to keep this code since it is more explicit in terms of distinguishing case 2 and 3. Line 121: private boolean areComponentsMergable(List<ILSMComponent> immutableComponents) { > You may add this as a static method in ILSMMergePolicy and use it on both p Definition of mergable components are different for each policy. So, this seems better be in each merge policy. Line 135: * @return the index of the latest component being merged among the given list of immutable components > the @return description needs to be fixed. Done Line 137: private boolean isMergeOngoing(List<ILSMComponent> immutableComponents) { > You may add this as a static method in ILSMMergePolicy and use it on both p It seems not appropriate to put this method in the interface level. If there is a helper class, I would do that. But, it seems overkill to create a helper class for a single method. https://asterix-gerrit.ics.uci.edu/#/c/795/3/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/PrefixMergePolicy.java File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/PrefixMergePolicy.java: Line 52: ILSMIndexAccessor accessor = (ILSMIndexAccessor) index.createAccessor(NoOpOperationCallback.INSTANCE, > Remove (ILSMIndexAccessor) cast Done Line 111: // case 1. > remove WSs and use multiline comment Done Line 136: // [case 1] > Something like this is cleaner: I want to keep explicit separation for each case. Line 165: * > WS Done Line 167: * @return the index of the latest component being merged among the given list of immutable components > @return description needs to be fixed. Done Line 252: ILSMIndexAccessor accessor = (ILSMIndexAccessor) index.createAccessor(NoOpOperationCallback.INSTANCE, > Remove (ILSMIndexAccessor) cast Done -- To view, visit https://asterix-gerrit.ics.uci.edu/795 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ide99c022861f96cd60bc8f5795c4964ab02b3e14 Gerrit-PatchSet: 3 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Young-Seok Kim <kiss...@gmail.com> Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Gerrit-Reviewer: Murtadha Hubail <hubail...@gmail.com> Gerrit-Reviewer: Young-Seok Kim <kiss...@gmail.com> Gerrit-HasComments: Yes
