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