> On March 20, 2014, 4:55 p.m., Sean Busbey wrote: > > server/base/src/main/java/org/apache/accumulo/server/conf/ZooConfiguration.java, > > lines 66-75 > > <https://reviews.apache.org/r/19142/diff/3/?file=520527#file520527line66> > > > > This "just pick one instance id" code is already repeated. probably > > worth pulling it to a common location. > > > > Maybe into the Accumulo class similar to the > > "getAccumuloPersistedVersion" call? Or as a method on VolumeManager?
I considered doing this, but opted against it for a few reasons. Most importantly, if we're moving from the concept of a single Volume (or FileSystem) to multiple, it doesn't make sense to allow a dev to write with only one in mind. The cases where we do this are specific to the task at hand being agnostic of multiple Volumes (in this case, assumption of instance_id being uniform). For choosing which Volume a file should go on, there's already the VolumeChooser and (RandomVolumeChooser impl). Unless you can think of a good reason for this, I'd prefer to not encapsulate this to avoid encouraging bad practice. > On March 20, 2014, 4:55 p.m., Sean Busbey wrote: > > server/base/src/main/java/org/apache/accumulo/server/conf/ZooConfiguration.java, > > lines 64-67 > > <https://reviews.apache.org/r/19142/diff/3/?file=520527#file520527line64> > > > > Do we already have a startup check to validate this assumption? I had thought so, but it doesn't appear so (or at least I can't find it again). Inside of Accumulo.java sounds like a good place for it. - Josh ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19142/#review37904 ----------------------------------------------------------- On March 16, 2014, 3:43 a.m., Josh Elser wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/19142/ > ----------------------------------------------------------- > > (Updated March 16, 2014, 3:43 a.m.) > > > Review request for accumulo, Eric Newton and kturner. > > > Bugs: ACCUMULO-2061 > https://issues.apache.org/jira/browse/ACCUMULO-2061 > > > Repository: accumulo > > > Description > ------- > > Removes instance.dfs.dir from being appended to the configured > instance.volumes values. Removed FileSystem from the VolumeManager in favor > of a new Volume class. > > The Volume class contains a FileSystem but also a base path which is denotes > what the Accumulo directory is on that FileSystem. > > > Diffs > ----- > > > core/src/main/java/org/apache/accumulo/core/client/admin/TableOperationsImpl.java > 0b2f10e > core/src/main/java/org/apache/accumulo/core/client/impl/OfflineScanner.java > c90d380 > core/src/main/java/org/apache/accumulo/core/conf/Property.java fc4d012 > core/src/main/java/org/apache/accumulo/core/file/VolumeConfiguration.java > fb8c6c8 > core/src/main/java/org/apache/accumulo/core/file/rfile/PrintInfo.java > 4cfefad > > core/src/main/java/org/apache/accumulo/core/file/rfile/bcfile/PrintInfo.java > f21190e > core/src/main/java/org/apache/accumulo/core/util/shell/Shell.java 0c2cf3c > core/src/main/java/org/apache/accumulo/core/volume/NonConfiguredVolume.java > PRE-CREATION > core/src/main/java/org/apache/accumulo/core/volume/Volume.java PRE-CREATION > core/src/main/java/org/apache/accumulo/core/volume/VolumeConfiguration.java > PRE-CREATION > core/src/main/java/org/apache/accumulo/core/volume/VolumeImpl.java > PRE-CREATION > core/src/main/java/org/apache/accumulo/core/zookeeper/ZooUtil.java de1b432 > > core/src/test/java/org/apache/accumulo/core/volume/NonConfiguredVolumeTest.java > PRE-CREATION > server/base/src/main/java/org/apache/accumulo/server/Accumulo.java 2fa9051 > server/base/src/main/java/org/apache/accumulo/server/ServerConstants.java > 8983d08 > > server/base/src/main/java/org/apache/accumulo/server/client/BulkImporter.java > dc9acf8 > > server/base/src/main/java/org/apache/accumulo/server/client/HdfsZooInstance.java > 6993a0a > > server/base/src/main/java/org/apache/accumulo/server/conf/ZooConfiguration.java > 32f6126 > server/base/src/main/java/org/apache/accumulo/server/fs/VolumeManager.java > f0c7083 > > server/base/src/main/java/org/apache/accumulo/server/fs/VolumeManagerImpl.java > 80301ef > server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java > da3baa6 > server/base/src/main/java/org/apache/accumulo/server/init/Initialize.java > 925f602 > > server/base/src/main/java/org/apache/accumulo/server/master/recovery/HadoopLogCloser.java > 7edc0cf > > server/base/src/main/java/org/apache/accumulo/server/master/recovery/MapRLogCloser.java > bba7ac5 > server/base/src/main/java/org/apache/accumulo/server/util/ChangeSecret.java > ac13034 > server/base/src/main/java/org/apache/accumulo/server/util/FileUtil.java > 8e38cbd > > server/base/src/main/java/org/apache/accumulo/server/util/LocalityCheck.java > a96e791 > > server/base/src/main/java/org/apache/accumulo/server/util/TabletOperations.java > b237cd0 > > server/base/src/main/java/org/apache/accumulo/server/util/ZooKeeperMain.java > 37edb1a > > server/base/src/test/java/org/apache/accumulo/server/ServerConstantsTest.java > a316155 > server/base/src/test/java/org/apache/accumulo/server/fs/FileTypeTest.java > 205a793 > server/base/src/test/java/org/apache/accumulo/server/fs/VolumeUtilTest.java > c85be45 > server/gc/src/test/java/org/apache/accumulo/gc/GarbageCollectionTest.java > 4e2a878 > > server/master/src/main/java/org/apache/accumulo/master/tableOps/ExportTable.java > 36bbb53 > > server/master/src/main/java/org/apache/accumulo/master/tableOps/ImportTable.java > 7e84c55 > > server/monitor/src/main/java/org/apache/accumulo/monitor/servlets/DefaultServlet.java > 942f866 > > server/tserver/src/main/java/org/apache/accumulo/tserver/BulkFailedCopyProcessor.java > e9f1083 > server/tserver/src/main/java/org/apache/accumulo/tserver/Compactor.java > 151db6e > server/tserver/src/main/java/org/apache/accumulo/tserver/FileManager.java > bb95532 > server/tserver/src/main/java/org/apache/accumulo/tserver/Tablet.java > cc4b68d > server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java > 475621b > > server/tserver/src/main/java/org/apache/accumulo/tserver/compaction/MajorCompactionRequest.java > 3bbb476 > server/tserver/src/main/java/org/apache/accumulo/tserver/log/LogSorter.java > 8f783c3 > > server/tserver/src/main/java/org/apache/accumulo/tserver/log/MultiReader.java > a28bac4 > server/tserver/src/test/java/org/apache/accumulo/tserver/RootFilesTest.java > 1cd8f12 > > server/tserver/src/test/java/org/apache/accumulo/tserver/TabletServerSyncCheckTest.java > 50c8b31 > > server/tserver/src/test/java/org/apache/accumulo/tserver/log/MultiReaderTest.java > c4d3dfb > > server/tserver/src/test/java/org/apache/accumulo/tserver/log/SortedLogRecoveryTest.java > 359bfa1 > > server/tserver/src/test/java/org/apache/accumulo/tserver/log/TestUpgradePathForWALogs.java > af149fa > > test/src/main/java/org/apache/accumulo/test/performance/scan/CollectTabletStats.java > 9a9cad7 > test/src/test/java/org/apache/accumulo/test/VolumeIT.java a0efe45 > test/src/test/java/org/apache/accumulo/test/functional/BulkFileIT.java > c8023c0 > > Diff: https://reviews.apache.org/r/19142/diff/ > > > Testing > ------- > > All unit and integration tests run. Tested locally with multiple volumes on a > single HDFS. Tested upgrade from 1.5 with volumes (verified that relative > paths were rewritten as data was ingested). Tested volume replacement on a > new instance. All "real instance" tests used CI to generate data. > > > Thanks, > > Josh Elser > >
