Jianfeng Jia has posted comments on this change. Change subject: Avoid always merging old components in prefix policy ......................................................................
Patch Set 4: (7 comments) some minor comments. https://asterix-gerrit.ics.uci.edu/#/c/1818/4/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 49: private final static double MAX_MERGABLE_COMPONENT_SIZE_RATIO = 1.2; maybe it's good to also put the source of this magic number? Line 143: Collections.reverse(immutableComponents); could you explain the reason of the reverse? Line 256: private Pair<Integer, Integer> getMergableComponentsIndex(List<ILSMDiskComponent> immutableComponents) { I think it's better to move this big chunk of comment before the function as Java doc? Line 266: || immutableComponents.get(i).getState() != ComponentState.READABLE_UNWRITABLE) { just curious, to make sure the state check is consistent, is this function (or its caller) synchronized or there is only one merge thread who also takes care of updating the state? Line 276: // normally this shouldn't happen, but just make sure we skip unmergeable components if it's a suspicious state, it's better to add a warning log. Line 284: boolean isLastComponent = j + 1 == immutableComponents.size() ? true : false; no need to use ternary? Line 289: return Pair.of(i, j); it feels like we should treat two part of the && differently for the else case? if `!canMerge` [i,j], we just increase j, if `canMerge` [i,j], if `shouldMerge[i,j]`, return else increase i and break correct me if I'm wrong :-) -- To view, visit https://asterix-gerrit.ics.uci.edu/1818 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I464da3fed38cded0aee7b319a35664eae069a2ba Gerrit-PatchSet: 4 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: Yingyi Bu <[email protected]> Gerrit-Reviewer: abdullah alamoudi <[email protected]> Gerrit-HasComments: Yes
