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 26507: ACCUMULO-3177 Create a per table volume chooser and ACCUMULO-3178 Create example preferred volumes chooser
Date Mon, 13 Oct 2014 18:05:45 GMT

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


Noticed some extra things that I didn't on Github. I also tried to not repeat things I had
already commented on over there (for both your and my sanity).


server/base/src/main/java/org/apache/accumulo/server/fs/PerTableVolumeChooser.java
<https://reviews.apache.org/r/26507/#comment96699>

    Why not import HashMap?



server/base/src/main/java/org/apache/accumulo/server/fs/PerTableVolumeChooser.java
<https://reviews.apache.org/r/26507/#comment96700>

    Include a message that states that the PerTableVolumeChooser failed and that's falling
back to random.



server/base/src/main/java/org/apache/accumulo/server/fs/RandomVolumeChooser.java
<https://reviews.apache.org/r/26507/#comment96703>

    Logging here could get pretty spammy on an active node. We should be able to extrapolate
what the chosen volume was when the file actually gets written. Maybe pull them back to trace
or just drop.



server/base/src/main/java/org/apache/accumulo/server/fs/VolumeChooser.java
<https://reviews.apache.org/r/26507/#comment96704>

    Keep the original method and add some way for implementations to get a VolumeChooserEnvironment
or get rid of the original method. Having both is unnecessary and confusing.



server/base/src/main/java/org/apache/accumulo/server/fs/VolumeManagerImpl.java
<https://reviews.apache.org/r/26507/#comment96706>

    Would be better to just make a VolumeChooserEnvironment that doesn't have any data (to
use when !tableId.isPresent()). That collapses this down into the single choose(..) method.



server/base/src/main/java/org/apache/accumulo/server/fs/VolumeManagerImpl.java
<https://reviews.apache.org/r/26507/#comment96705>

    Needs log message for exception


- Josh Elser


On Oct. 9, 2014, 5:23 p.m., Jenna Huston wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26507/
> -----------------------------------------------------------
> 
> (Updated Oct. 9, 2014, 5:23 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-3177 and ACCUMULO-3178
>     https://issues.apache.org/jira/browse/ACCUMULO-3177
>     https://issues.apache.org/jira/browse/ACCUMULO-3178
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Added a per table volume chooser that allows tables to be given a specific volume chooser.
 The second patch, ACCUMULO-3178, adds an example, a preferred volume chooser which gives
the preferred volume for a table.  When a table chooser is not specified, or a preferred volume
is not specified then, the default chooser is the RandomVolumeChooser.
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/conf/Property.java ad4fe92 
>   server/base/src/main/java/org/apache/accumulo/server/fs/PerTableVolumeChooser.java
PRE-CREATION 
>   server/base/src/main/java/org/apache/accumulo/server/fs/RandomVolumeChooser.java 2760b07

>   server/base/src/main/java/org/apache/accumulo/server/fs/VolumeChooser.java 8713c97

>   server/base/src/main/java/org/apache/accumulo/server/fs/VolumeChooserEnvironment.java
PRE-CREATION 
>   server/base/src/main/java/org/apache/accumulo/server/fs/VolumeManager.java cbfdb5e

>   server/base/src/main/java/org/apache/accumulo/server/fs/VolumeManagerImpl.java b2341b6

>   server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java 82b77ee 
>   server/base/src/main/java/org/apache/accumulo/server/init/Initialize.java 4305e71 
>   server/base/src/main/java/org/apache/accumulo/server/util/FileUtil.java 0f7ac22 
>   server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java 8d14cad

>   server/base/src/main/java/org/apache/accumulo/server/util/TabletOperations.java b8e7113

>   server/master/src/main/java/org/apache/accumulo/master/Master.java b435b0f 
>   server/master/src/main/java/org/apache/accumulo/master/TabletGroupWatcher.java 0a3d1d0

>   server/master/src/main/java/org/apache/accumulo/master/tableOps/CreateTable.java f56c1e2

>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/DfsLogger.java e923ebc

>   server/tserver/src/test/java/org/apache/accumulo/tserver/TabletServerSyncCheckTest.java
dad9a75 
> 
> Diff: https://reviews.apache.org/r/26507/diff/
> 
> 
> Testing
> -------
> 
> New IT in the patch for ACCUMULO-3178.  Could not test ACCUMULO-3177 without an example
chooser.
> 
> 
> File Attachments
> ----------------
> 
> Diff for 3178
>   https://reviews.apache.org/media/uploaded/files/2014/10/09/07d2693e-9acc-438b-9b13-667bde467590__0001-ACCUMULO-3178-Create-example-preferred-volumes-choos.patch
> 
> 
> Thanks,
> 
> Jenna Huston
> 
>


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