Ian Maxon has posted comments on this change. Change subject: ASTERIXDB-1479: Change storage valid int and add explicit version ......................................................................
Patch Set 8: (5 comments) https://asterix-gerrit.ics.uci.edu/#/c/919/8/hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/frames/BTreeFieldPrefixNSMLeafFrame.java File hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/frames/BTreeFieldPrefixNSMLeafFrame.java: Line 464: strBuilder.append("TUPLE_COUNT_OFF: " + tupleCountOff + "\n"); > Should we revert this? Not sure why we want to change the name of the disp Yeah I can revert this, it was just the IntelliJ refactor plugin being a bit overzealous. https://asterix-gerrit.ics.uci.edu/#/c/919/8/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/freepage/LinkedMetaDataPageManager.java File hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/freepage/LinkedMetaDataPageManager.java: Line 475: return (long)(metadataPageNum * bufferCache.getPageSize()) + LIFOMetaDataFrame.LSN_OFF; > Should one of the operands to the multiply be casted to a long rather than Well it's fixed as far as sonarQube is concerned :P it did complain at first. It only complained because I changed the name here, so it's been a bug for a while apparently. Doesn't the output of metadataPageNum*getPageSize() being casted to a long guarantee the result of the subsequent addition is also a long? https://asterix-gerrit.ics.uci.edu/#/c/919/8/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: Line 53: public enum TREE_INDEX_STATE { > enums should be named like classes (e.g. TreeIndexState)- not sure why this Done Line 55: WRONG_VERS, > Not loving this abbreviation- can we have WRONG_VERSION or perhaps VERSION_ Done Line 105: return TREE_INDEX_STATE.WRONG_VERS; > We may have to revisit this in the future- we're going to need to know whic The index metadata can still be opened though to read what version it is. Right now the error/version mismatch behavior is actually that the LSM harness will not open the index because there are a mismatched # of bloom filter and index files (because the _b part of the component won't open due to this code). -- To view, visit https://asterix-gerrit.ics.uci.edu/919 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I888ff0eacf5b3cb6ad7ec002c74f113c6ffcd496 Gerrit-PatchSet: 8 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Ian Maxon <[email protected]> Gerrit-Reviewer: Ian Maxon <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Michael Blow <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-Reviewer: abdullah alamoudi <[email protected]> Gerrit-HasComments: Yes
