abdullah alamoudi has posted comments on this change.

Change subject: Improve Reading from and Writing to Metadata Pages
......................................................................


Patch Set 2:

(5 comments)

https://asterix-gerrit.ics.uci.edu/#/c/1476/2/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/CorrelatedPrefixMergePolicy.java
File 
asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/CorrelatedPrefixMergePolicy.java:

PS2, Line 67: AbstractDiskLSMComponent
> Why Abstract instead of the interface?
It doesn't make much difference. I am just removing an unneeded upcast.


https://asterix-gerrit.ics.uci.edu/#/c/1476/2/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:

PS2, Line 48:            if (opType == LSMOperationType.MERGE) {
            :                 LongPointable markerLsn = 
LongPointable.FACTORY.createPointable(ComponentMetadataUtils
            :                         
.getLong(oldComponents.get(0).getMetadata(), 
ComponentMetadataUtils.MARKER_LSN_KEY, -1L));
            :                 
btreeComponent.getMetadata().put(ComponentMetadataUtils.MARKER_LSN_KEY, 
markerLsn);
            :             }
> What's this about?
this used to be inside the merge method call. Basically, it takes the lsn of 
the marker from the last disk component and place it in the new component.

Basically, The code of the IO callbacks should decides how content of metadata 
pages are merged into the new component.


https://asterix-gerrit.ics.uci.edu/#/c/1476/2/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/transactions/PrimaryIndexLogMarkerCallback.java
File 
asterixdb/asterix-common/src/main/java/org/apache/asterix/common/transactions/PrimaryIndexLogMarkerCallback.java:

PS2, Line 52: 
> Why's this in the callback now?
so this is a callback that places a marker lsn in the metadata page.

This lsn takes you to a marker log in the transaction file. each marker log 
takes you to the previous marker log.

So in the before, we used to get the marker lsn from the index using the 
removed method getMostRecentMarkerLSN which breaks the generality of the 
metadata page and add some code to the lsm btree that it doesn't need.

So, we put the code to get the Marker LSN from the metadata pages here. and 
made it self contained without touching index code.


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

PS2, Line 55:  if (!(o instanceof Triple<?, ?, ?>)) {
> Why'd you rid Pair of the instanceof() check but not Triple?
I did the check using the getClass() but shouldn't have. (btw, code generated 
by the IDE, will fix)


https://asterix-gerrit.ics.uci.edu/#/c/1476/2/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:

PS2, Line 91:  if (bytes == null) {
            :             start = 0;
            :             length = TYPE_TRAITS.getFixedLength();
            :             bytes = new byte[length];
            :         }
> Why ?
otherwise, it will throw a nullpointer exception.


-- 
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: 2
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