asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "abdullah alamoudi (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 19:32:15 GMT
abdullah alamoudi has posted comments on this change.

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


Patch Set 6:

(10 comments)

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?
Yes


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:
I am looking at your patch. We can prevent this scenario you're describing through ensuring
all memory components of a partition are activated without allowing scheduleFlush to happen
in between.


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?
We shouldn't.
This is because if the cursor has gone past the end of the in memory component, then the includeMutableComponent
will be set to false.
Still, when that happens, we want to exit the memory component to allow it to be recycled
and used for modifications.

Added a comment


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 
Yes. realized this while I was working on the change and was lazy to change it at that time.

Done.


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?
This case is handled okay.
When this happen, the cursor will basically re-enter the same memory component and will not
attempt to switch components during the current search operation.

Added a test case for this.


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


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?
We can remove it at some point but I don't think it cost much and is useful to show contention
over the opTracker and the components


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


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


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?
I just thought of this number as not too bad of a number that would work well with leaf pages
that take about 1000 records each.
I can file an issue for someone to come up with a better way to do this. we can also do a
time based thing.
A better idea?


-- 
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-Reviewer: abdullah alamoudi <bamousaa@gmail.com>
Gerrit-HasComments: Yes

Mime
View raw message