accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ke...@deenlo.com
Subject Re: Review Request 22658: ACCUMULO-2889: Batch metadata updates for new WALs
Date Mon, 30 Jun 2014 16:56:27 GMT


> On June 30, 2014, 3:24 p.m., kturner wrote:
> > server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java,
line 267
> > <https://reviews.apache.org/r/22658/diff/2/?file=619604#file619604line267>
> >
> >     would be interesting to see performance diff w/ this change and ACCUMULO-2801
> >
> 
> Jonathan Park wrote:
>     Bump on the comments on that ticket :)

I made some comments on it.


> On June 30, 2014, 3:24 p.m., kturner wrote:
> > server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java,
line 262
> > <https://reviews.apache.org/r/22658/diff/2/?file=619604#file619604line262>
> >
> >     Maybe this is a problem in the existing code, but this change may make the problem
worse.  Calling beginUpdatingLogsUsed() updates the tablets in memory data structs to indicate
the tablet is using the log.  If the code after this point fails, then all tablets will think
the walog is in the metadata table even though it is not.  
> >     
> >
> 
> Jonathan Park wrote:
>     I guess you're suggesting that if we failed for whatever reason while holding the
lock, our changes shouldn't be published outside of the lock (since all readers of the wal
in memory structs also lock).
>     
>     I think we could either implement some kind of rollback or convince ourselves that
it's harmless (or implement it so that it is harmless). I'll give this some thought.
> 
> kturner wrote:
>     Maybe the tablet data structs should be updated in finishUpdatingLogsUsed() after
the metadata updates are made successfully.  Then have to consider the case where walog ref
ends up in metadata table but not in tablet data structs.  Seems like this case is easier
to prevent and should it occur it seems innocuous.

I opened ACCUMULO-2960 to track this possible issue.


- kturner


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


On June 27, 2014, 5:41 p.m., Jonathan Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22658/
> -----------------------------------------------------------
> 
> (Updated June 27, 2014, 5:41 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/InternalBatchWriter.java
PRE-CREATION 
>   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 57415bd

>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java
d25ee75 
>   test/src/test/java/org/apache/accumulo/server/util/InternalBatchWriterIT.java PRE-CREATION

>   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