-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22658/#review45871
-----------------------------------------------------------



core/src/main/java/org/apache/accumulo/core/client/impl/Writer.java
<https://reviews.apache.org/r/22658/#comment80933>

    I'm not quite sure I understand the use of a ThreadLocal here. I'm thinking 
about a case where I have multiple tablets on a tserver. I have two writes 
coming in with updates to that same tserver, one to each tablet. Aren't those 
updates going to be running in their own threads anyways, so we wouldn't 
actually see any batching happening?
    
    If I'm missing something, please point me at some code and I'd be happy to 
try to understand better myself the context.



core/src/main/java/org/apache/accumulo/core/client/impl/Writer.java
<https://reviews.apache.org/r/22658/#comment80923>

    Do we want to spin faster than half a second?



core/src/main/java/org/apache/accumulo/core/client/impl/Writer.java
<https://reviews.apache.org/r/22658/#comment80931>

    Again, is 500ms too long to spin before retrying?



server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java
<https://reviews.apache.org/r/22658/#comment80918>

    Do we want to be batching updates to the root table? We should really be 
updating the root table about log(n) as often as we update the metadata table, 
right?


First pass at looking over this. I need to give it a few more looks (with 
proper external context). For the next patch, can you make sure to run the new 
code through the formatter too, please? I noticed a few places where you have 
your own style :).

- Josh Elser


On June 16, 2014, 9:59 p.m., Jonathan Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22658/
> -----------------------------------------------------------
> 
> (Updated June 16, 2014, 9:59 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-2889
>     https://issues.apache.org/jira/browse/ACCUMULO-2889
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Added additional methods to the Writer class to handle batching. 
> 
> Potential risks are that we're now holding onto the locks for a bit longer 
> than we used to. All tablets present in a batch will have their logLock's 
> locked until the batch is complete. 
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/client/impl/Writer.java d6762e7 
>   
> server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java
>  374017d 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/Tablet.java 
> 36b2289 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java 
> e2510d7 
>   
> server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java
>  d25ee75 
>   test/src/test/java/org/apache/accumulo/test/BatchMetadataUpdatesIT.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/22658/diff/
> 
> 
> Testing
> -------
> 
> Added a new IT.
> 
> We insert a few entries to tablets and ensure that the relevant entries 
> appear in the WAL and Metadata Table.
> One test case that isn't included yet is verifying that root + metadata table 
> entries are entered correctly.
> 
> 
> Thanks,
> 
> Jonathan Park
> 
>

Reply via email to