asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Michael Blow (Code Review)" <do-not-re...@asterixdb.incubator.apache.org>
Subject Change in asterixdb[master]: ASTERIXDB-1479: Change storage valid int and add explicit ve...
Date Tue, 14 Jun 2016 15:07:04 GMT
Michael Blow has posted comments on this change.

Change subject: ASTERIXDB-1479: Change storage valid int and add explicit version
......................................................................


Patch Set 8:

(6 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 displayed keys only.
 If we're keeping, we probably should fix the spacing so that the values align with the others.


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/frames/TreeIndexNSMFrame.java
File hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/frames/TreeIndexNSMFrame.java:

Line 284:         strBuilder.append("TUPLE_COUNT_OFF:     " + tupleCountOff + "\n");
same comment as before


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 the product?  I
think otherwise we risk overflow.


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 isn't caught by
SonarQube


Line 55:         WRONG_VERS,
Not loving this abbreviation- can we have WRONG_VERSION or perhaps VERSION_MISMATCH?


Line 105:                     return TREE_INDEX_STATE.WRONG_VERS;
We may have to revisit this in the future- we're going to need to know which specific version
the index file has in order to provide the detailed error message Till was proposing, I think.


-- 
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 <imaxon@apache.org>
Gerrit-Reviewer: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Michael Blow <michael.blow@couchbase.com>
Gerrit-Reviewer: Till Westmann <tillw@apache.org>
Gerrit-Reviewer: abdullah alamoudi <bamousaa@gmail.com>
Gerrit-HasComments: Yes

Mime
View raw message