Luo Chen has posted comments on this change. Change subject: [ASTERIXDB-2414][STO] Fix name of merge files ......................................................................
Patch Set 5: (1 comment) https://asterix-gerrit.ics.uci.edu/#/c/2820/4/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/AbstractLSMIndexFileManager.java File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/AbstractLSMIndexFileManager.java: PS4, Line 212: String end = lastTimestampRange[1]; : if (end.compareTo(start) <= 0) { : throw new IllegalArgumentException( : "A Merge file must have end greater than start. Found end: " + end + " and start: " + start); : } : // Get the range of timestamps by taking the earliest and the latest timestamps : return new LSMComponentFileReferences(baseDir.getChild(start + DELIMITER + end), null, null); : } : > Mmmm, how would it break storage? The comparison here should be based on Long instead of String (why alphabetically?) For backward-compatibility, consider some disk components of the previous storage layout, e.g., components named 5-1, 10-7, 15-11, 16-16 (ordered from oldest to newest). With the previous naming convention, merging the first three components would give 15-1. However, with the new change, this would always throw the illegal argument exception... I think the bug (if any) is caused by the inconsistency between the file naming here and the one used during recovery. Changing either one would be OK to me, but just to make sure to warn users if this breaks the storage... -- To view, visit https://asterix-gerrit.ics.uci.edu/2820 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I861765bc0f293bdfdf0285f97884d536204fdb1e Gerrit-PatchSet: 5 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: abdullah alamoudi <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Dmitry Lychagin <[email protected]> Gerrit-Reviewer: Ian Maxon <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Luo Chen <[email protected]> Gerrit-Reviewer: Michael Blow <[email protected]> Gerrit-Reviewer: Murtadha Hubail <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-Reviewer: Wail Alkowaileet <[email protected]> Gerrit-Reviewer: abdullah alamoudi <[email protected]> Gerrit-HasComments: Yes
