Ian Maxon has posted comments on this change.

Change subject: Implemented Disk Components Alignment Based on IDs
......................................................................


Patch Set 4:

(4 comments)

A few questions

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

PS4, Line 105: 
             :         // [case 1]
Can there be a toplevel comment describing what case 1/2/3 are?


PS4, Line 180:  private void triggerScheduledMerge(long minID, long maxID, int 
partition, Set<IndexInfo> indexInfos)
Why call this with a set of indices, and the partition, and filter out the 
indices which don't apply from the merge, rather than only populating 
indexInfos with those that are relevant?


https://asterix-gerrit.ics.uci.edu/#/c/1761/4/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/ioopcallbacks/AbstractLSMIOOperationCallback.java
File 
asterixdb/asterix-common/src/main/java/org/apache/asterix/common/ioopcallbacks/AbstractLSMIOOperationCallback.java:

PS4, Line 114:  long id = getComponentLSN(oldComponents);
Wait, what? Why getComonentLSN of a null value?


https://asterix-gerrit.ics.uci.edu/#/c/1761/4/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/AbstractLSMDiskComponent.java
File 
hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/AbstractLSMDiskComponent.java:

PS4, Line 108:     //TODO: do we need to throw an exception when ID is not 
found?
If you can make it so that it fails safely that would be good. This way the 
metadata change will be non-breaking.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I768ee9ac0a8d3c99c631086093a6b778b2e7588e
Gerrit-PatchSet: 4
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Luo Chen <[email protected]>
Gerrit-Reviewer: Ian Maxon <[email protected]>
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-Reviewer: Jianfeng Jia <[email protected]>
Gerrit-Reviewer: Till Westmann <[email protected]>
Gerrit-Reviewer: Yingyi Bu <[email protected]>
Gerrit-Reviewer: abdullah alamoudi <[email protected]>
Gerrit-HasComments: Yes

Reply via email to