Luo Chen has posted comments on this change.

Change subject: [NO ISSUE][STO] Add consistency to flush lifecycle
......................................................................


Patch Set 16:

(8 comments)

Some minor comments...

https://asterix-gerrit.ics.uci.edu/#/c/2584/16/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/BaseOperationTracker.java
File 
asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/BaseOperationTracker.java:

PS16, Line 40: 
             : 
Why FLUSH/MERGE is removed?


https://asterix-gerrit.ics.uci.edu/#/c/2584/16/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/ioopcallbacks/LSMIOOperationCallback.java
File 
asterixdb/asterix-common/src/main/java/org/apache/asterix/common/ioopcallbacks/LSMIOOperationCallback.java:

PS16, Line 65: ArrayDeque
Use Deque interface?


PS16, Line 177: componentId
component LSN


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

PS16, Line 143:     String getOperationGroupId();
What's the usage of this method? More comments about this "group"?


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

PS16, Line 62: map
use a more meaning name, like parameterMap?


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

PS16, Line 68:                         PriorityQueue<ILSMIOOperation> q = new 
PriorityQueue<>();
Why we need a priority queue here? It seems FIFO queue should be enough?


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

PS16, Line 32:     private int currentComponentIndex;
Why do we need to keep track of this?


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

PS16, Line 38:  // Note that by using a flush target file name, we state that 
the
             :         // new bulk loaded component is "newer" than any other 
merged component.
This comment can be removed?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I29f7992ec6c0f71c5b63d45800b2fb590d651e4b
Gerrit-PatchSet: 16
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: abdullah alamoudi <bamou...@gmail.com>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Luo Chen <cl...@uci.edu>
Gerrit-Reviewer: Murtadha Hubail <mhub...@apache.org>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-Reviewer: abdullah alamoudi <bamou...@gmail.com>
Gerrit-HasComments: Yes

Reply via email to