accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Josh Elser" <josh.el...@gmail.com>
Subject Re: Review Request 22658: ACCUMULO-2889: Batch metadata updates for new WALs
Date Tue, 17 Jun 2014 00:38:44 GMT

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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message