abdullah alamoudi has posted comments on this change.

Change subject: Improve reading from and writing to Metadata pages
......................................................................


Patch Set 10:

(8 comments)

https://asterix-gerrit.ics.uci.edu/#/c/1476/10/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/ioopcallbacks/LSMBTreeIOOperationCallback.java
File 
asterixdb/asterix-common/src/main/java/org/apache/asterix/common/ioopcallbacks/LSMBTreeIOOperationCallback.java:

PS10, Line 49: .
> Object creation -- not sure this is a frequent code path or not?
This should be very infrequent. Each time we do a merge.


https://asterix-gerrit.ics.uci.edu/#/c/1476/7/hyracks-fullstack/algebricks/algebricks-common/src/main/java/org/apache/hyracks/algebricks/common/utils/Pair.java
File 
hyracks-fullstack/algebricks/algebricks-common/src/main/java/org/apache/hyracks/algebricks/common/utils/Pair.java:

PS7, Line 53: 
> returns Objects.hash(...)
Done


https://asterix-gerrit.ics.uci.edu/#/c/1476/10/hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/primitive/LongPointable.java
File 
hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/primitive/LongPointable.java:

PS10, Line 58: pointable
> just call setLong here?
Done


https://asterix-gerrit.ics.uci.edu/#/c/1476/7/hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/util/ArrayBackedValueStorage.java
File 
hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/util/ArrayBackedValueStorage.java:

PS7, Line 78: 
> return Objects.hash(..)
Done


https://asterix-gerrit.ics.uci.edu/#/c/1476/7/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/utils/ComponentMetadataUtil.java
File 
hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/utils/ComponentMetadataUtil.java:

PS7, Line 56:             throws HyracksDataException {
> Can you put a corresponding writeLong(...) method in this Util to pair with
Mmmm, that would be tricky. I can do something like:

public static void putLong(IComponentMetadata metadata, IValueReference key, 
long value)
            throws HyracksDataException {
        metadata.put(key, LongPointable.FACTORY.createPointable(
}

but this will create a byte array of size 8 with each call. I can also do:

public static void putLong(IComponentMetadata metadata, IValueReference key, 
IValueReference value)
            throws HyracksDataException {
        metadata.put(key, value);
}

but then why not call the method directly... I think let's keep it without the 
putLong() and add it if needed?


PS7, Line 75: putLong
> Why do you need this second getLong(...) method?
Done


PS7, Line 91: rac
> It looks get(...) is only used in LogMarkerTest, why not move get(..), from
We need it in other cases for some extensions and it is generic enough and 
useful enough to keep in a Util class.


https://asterix-gerrit.ics.uci.edu/#/c/1476/7/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/impls/LSMInvertedIndex.java
File 
hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/impls/LSMInvertedIndex.java:

PS7, Line 1011: cksDataException.create
> Error code?
Done


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1476
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id95ef33c0a0bc1abb3fc3ecdea5611ee4acd6dfa
Gerrit-PatchSet: 10
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: abdullah alamoudi <[email protected]>
Gerrit-Reviewer: Ian Maxon <[email protected]>
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-Reviewer: Yingyi Bu <[email protected]>
Gerrit-Reviewer: abdullah alamoudi <[email protected]>
Gerrit-HasComments: Yes

Reply via email to