Luo Chen has posted comments on this change.

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


Patch Set 8:

(11 comments)

> (13 comments)

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

PS8, Line 68: ArrayList
> ArrayList -> List
Done


PS8, Line 82: ArrayList
> ArrayList->List
Done


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

PS8, Line 115: public
> public -> private ?
Done


PS8, Line 121: WARNING
> Should that be an exception?  Is there any case that happen?
This could happen for bulk loaded index. At the end() method of LSM index bulk 
loader (say LSMBTreeBulkLoader), afterOperation is called for that bulk loaded 
component (and bulk loaded is treated as flush), but the FLUSH_LSN is not set 
for bulk-loaded components.

If we can distinguish between FLUSH and BULK_LOAD operations, then we should 
throw an exception.


PS8, Line 141: public
> public->private
Done


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

Line 49: 
> Add java doc for the new interface method?
Done.
And I added java doc for ILSMDiskComponentID in that class (instead of this 
method).


PS8, Line 50: getComponentID
> ID-> Id
Done


PS8, Line 50: ILSMDiskComponentID
> ID->Id
Done


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

PS8, Line 109: LSMDiskComponentID
> Make ILSMDiskComponentID a class member and construct it once in the constr
The problem is that for new disk components (flush or merge target), id is 
missing when this component is being constructed. Thus, the id should be 
non-final, and the caller needs to call setComponentId at a later time. Is this 
acceptable?


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

PS8, Line 54: result
> return 31 * Long.hashCode(minId) + Long.hashCode(maxId)  ?
Done


PS8, Line 64:         if (obj == null) {
            :             return false;
            :         }
            :         if (getClass() != obj.getClass()) {
            :             return false;
            :         }
> replace 64 to 69 with
Done


-- 
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: 8
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: Luo Chen <[email protected]>
Gerrit-Reviewer: Till Westmann <[email protected]>
Gerrit-Reviewer: Yingyi Bu <[email protected]>
Gerrit-Reviewer: Yingyi Bu <[email protected]>
Gerrit-Reviewer: abdullah alamoudi <[email protected]>
Gerrit-HasComments: Yes

Reply via email to