asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Luo Chen (Code Review)" <do-not-re...@asterixdb.incubator.apache.org>
Subject Change in asterixdb[master]: [ASTERIXDB-2169][STO][TX] Unblock modifications during full ...
Date Mon, 20 Nov 2017 04:23:25 GMT
Luo Chen has posted comments on this change.

Change subject: [ASTERIXDB-2169][STO][TX] Unblock modifications during full scan
......................................................................


Patch Set 6:

(10 comments)

Looks nice! The implementation is pretty straightforward. A few small comments though.

https://asterix-gerrit.ics.uci.edu/#/c/2166/6/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/dataflow/ComponentRollbackTest.java
File asterixdb/asterix-app/src/test/java/org/apache/asterix/test/dataflow/ComponentRollbackTest.java:

PS6, Line 202: 
So these component ids are refreshed within rollback code?


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

PS6, Line 105:  // This component was just deleted, we refresh the component id, when it gets
recycled, it will get
             :                 // the new id from the component id generator.
             :                 // It is assumed that the component delete caller will ensure
that corresponding components in secondary
             :                 // indexes are deleted as well
             :                 idGenerator.refresh();
We need to discuss this to integrate with patch:
https://asterix-gerrit.ics.uci.edu/#/c/2153/.

The current way of maintaining component ids is not correct. Suppose we have two memory components
mem1 and mem2. Mem1 is flushed, and mem2 is activated. Then mem2 is flushed, but mem1 is still
being flushed. Then mem1 should get the component id that is refreshed by the flush of mem2.
However, we don't know when mem1 will finish flush, and it's possible that another partition
will trigger flush again, and thus mem1 will receive wrong id (especially if there are secondary
indexes...)


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

PS6, Line 97:  if (hasNextCallCount >= 
Can we add condition check includeMutableComponent here?


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

PS6, Line 156:  if (replacedComponentIds.size() < i + 1) {
             :                 replacedComponentIds.add(new LSMComponentId(((LSMComponentId)
components.get(i).getId()).getMinId(),
             :                         ((LSMComponentId) components.get(i).getId()).getMaxId()));
             :             } else {
             :                 replacedComponentIds.get(i).reset(((LSMComponentId) components.get(i).getId()).getMinId(),
             :                         ((LSMComponentId) components.get(i).getId()).getMaxId());
             :             }
Can we simply assign the id of the components to here? Since component ids are guaranteed
to be immutable, there is no need to create a copy of it. (BTW, reset method is only used
by the primary index search when it resets a component id from the input ).


PS6, Line 176:  if (!found) {
             :                 // component has been merged?
             :                 LOGGER.log(Level.WARNING, "Memory Component with id = " + replacedComponentIds.get(i)
             :                         + " was flushed and merged before search cursor replaces
it");
             :                 return false;
             :             }
I'm afraid of this as well... How is this case be handled?


PS6, Line 205:                     LOGGER.log(Level.INFO, "HUH!!!!! Removed a disk component
from the search operation");
Improve this message?


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

PS6, Line 93:  long before = System.nanoTime();
Do we really need a timer here for all enter components call?


PS6, Line 159:             ctx.incrementEnterExitTime(System.nanoTime() - before);
Same here.


PS6, Line 401: long before = System.nanoTime();
same here for the timer.


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

PS6, Line 40: SWITCH_COMPONENT_CYCLE = 100;
how is this number calculated?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I37ba52f6324ed1c5a78465c3a8cbcd351f1ed5bc
Gerrit-PatchSet: 6
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: abdullah alamoudi <bamousaa@gmail.com>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Luo Chen <cluo8@uci.edu>
Gerrit-HasComments: Yes

Mime
View raw message