abdullah alamoudi has posted comments on this change.

Change subject: Change the API for writing and reading metadata pages
......................................................................


Patch Set 6:

(9 comments)

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

Line 73:         return -1L;
> A named constant wasn't bad. Could we bring one back? (Not sure where the b
Ok :-\


https://asterix-gerrit.ics.uci.edu/#/c/1396/6/asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/management/ReplicationChannel.java
File 
asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/management/ReplicationChannel.java:

Line 346:                     if (afp.getLSNByteOffset() >= 0L) {
> Again, the named constant seems better.
We discussed this and I will use the constant


https://asterix-gerrit.ics.uci.edu/#/c/1396/6/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/ITreeIndexTupleReference.java
File 
hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/ITreeIndexTupleReference.java:

Line 29:     public void resetByTupleOffset(byte[] buf, int tupleStartOffset);
> What is the benefit of this interface change?
See the comment in the previous patch :)


https://asterix-gerrit.ics.uci.edu/#/c/1396/6/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 37:     protected static final int PAGE_LSN_OFFSET = 
Constants.RESERVED_HEADER_SIZE;
> I thought that LSNs were a Tx processing concept that shouldn't be needed/m
So, to remove this, we need to understand what is meant by LSN here. I am 
pretty sure it is something different from log sequence number.

We have LSN and NSN throughout the code for btree and rtree and we need to 
understand them well.


https://asterix-gerrit.ics.uci.edu/#/c/1396/6/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/freepage/AppendOnlyLinkedMetadataPageManager.java
File 
hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/freepage/AppendOnlyLinkedMetadataPageManager.java:

Line 306:         put(frameFactory.createFrame(), key, value);
> Do we actually allocate a new frame for every call?
Fixed the interface but we still create more frames than we should. I suggest 
we leave it to the next change?


Line 311:         get(createMetadataFrame(), key, value);
> Do we actually allocate a new frame for every call?
Done


https://asterix-gerrit.ics.uci.edu/#/c/1396/6/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/LSMBTreeDiskComponent.java
File 
hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/LSMBTreeDiskComponent.java:

Line 69:         treeMetaManager.get(MARKER_LSN_KEY, pointable);
> Why do we need a MARKER_LSN_KEY in Hyracks?
Let's remove it in the next change. should be easy to do but this is getting 
bigger than I'd like it to be :)


https://asterix-gerrit.ics.uci.edu/#/c/1396/6/hyracks-fullstack/hyracks/hyracks-storage-am-rtree/src/main/java/org/apache/hyracks/storage/am/rtree/frames/RTreeNSMFrame.java
File 
hyracks-fullstack/hyracks/hyracks-storage-am-rtree/src/main/java/org/apache/hyracks/storage/am/rtree/frames/RTreeNSMFrame.java:

Line 38:     protected static final int PAGE_NSN_OFFSET = 
TreeIndexNSMFrame.RESERVED_HEADER_SIZE;
> What is an NSN?
I have no idea.


Line 44:     private static final double DOUEBLE_EPSILON = 
computeDoubleEpsilon();
> s/DOUEBLE_EPSILON/DOUBLE_EPSILON/
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iadad522ab5568677aa816c74fc1d63acad505380
Gerrit-PatchSet: 6
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: Till Westmann <[email protected]>
Gerrit-Reviewer: abdullah alamoudi <[email protected]>
Gerrit-HasComments: Yes

Reply via email to