----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22658/#review45893 -----------------------------------------------------------
core/src/main/java/org/apache/accumulo/core/client/impl/Writer.java <https://reviews.apache.org/r/22658/#comment80983> If this was not static then you wouldn't need to pass instance, credentials, or configuration. Could be easier to unit test. core/src/main/java/org/apache/accumulo/core/client/impl/Writer.java <https://reviews.apache.org/r/22658/#comment80989> Lifecycle management: what happens if a user forgets to call commit? Maybe JavaDoc that it is important. core/src/main/java/org/apache/accumulo/core/client/impl/Writer.java <https://reviews.apache.org/r/22658/#comment80981> Why is this in a finally block? It is only true if the try block finished normally, so we can just call it. server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java <https://reviews.apache.org/r/22658/#comment80987> Infinite retry makes me uneasy. Especially with no back-off in between. server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java <https://reviews.apache.org/r/22658/#comment80988> Could be a single method? Implementations look very similar. server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java <https://reviews.apache.org/r/22658/#comment80990> What is the danger of holding on to locks for a long time here? Is it possible that other batches will be forced to wait a long time? - Mike Drob 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 > >
