asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Murtadha Hubail (Code Review)" <>
Subject Change in asterixdb[master]: [STO][IDX] Eliminated excess antimatter in LSMBTree
Date Fri, 25 Aug 2017 19:58:32 GMT
Murtadha Hubail has posted comments on this change.

Change subject: [STO][IDX] Eliminated excess antimatter in LSMBTree

Patch Set 10:


A general comment:
many downcasts were introduced due to the newly added BTree specific classes. Can we get rid
of these downcasts by making factories return BTree* type?
File hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/impls/

PS10, Line 26: FieldPrefixTupleReference
If this is used only for BTree, rename it to reflect this.
File hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/tuples/

PS10, Line 36: ITreeIndexTupleReference
Change the return type here to BTreeTypeAwareTupleWriter to avoid the downcasts.
File hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/

PS10, Line 22: interface
Add javadocs on each method
File hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/

PS10, Line 22: interface
File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/

PS10, Line 331: if (!isPrimaryIndex() && ((LSMBTreeTupleReference) scanCursor.getTuple()).isUpdated())
              :                     continue;
              :                 }
Can this be done in a better way where we pass false for updateAware in the primary case?
File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/tuples/

PS10, Line 61: double numBits = numFields + 1.0;
             :         if (updateAware) {
             :             numBits += 1.0;
             :         }
             :         return (int) Math.ceil(numBits / 8.0);
This is repeated in multiple places. It would be nice if it could be moved to a util class

PS10, Line 100: (1 << 6)
This should be a constant.
File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/utils/

PS10, Line 67: updateInPlaceAware
Please use consistent naming for this flag.
File hyracks-fullstack/hyracks/hyracks-tests/hyracks-storage-am-lsm-btree-test/src/test/java/org/apache/hyracks/storage/am/lsm/btree/util/

PS10, Line 77: isPrimary
I don't think adding a parameter called isPrimary in Hyracks storage level is a good idea.
I know that there is a method called isPrimaryIndex in LSMBTree but that should be renamed.

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I12a67eff8431b52d1f9051b793a5a64b15c009e9
Gerrit-PatchSet: 10
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Ildar Absalyamov <>
Gerrit-Reviewer: Ian Maxon <>
Gerrit-Reviewer: Ildar Absalyamov <>
Gerrit-Reviewer: Jenkins <>
Gerrit-Reviewer: Luo Chen <>
Gerrit-Reviewer: Murtadha Hubail <>
Gerrit-Reviewer: Till Westmann <>
Gerrit-Reviewer: abdullah alamoudi <>
Gerrit-HasComments: Yes

View raw message