accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sean Busbey" <s...@manvsbeard.com>
Subject Re: Review Request 19142: Deprecate instance.dfs.dir and instance.dfs.uri
Date Thu, 20 Mar 2014 20:41:43 GMT


> 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?
> 
> Josh Elser wrote:
>     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.

I see instance id and persisted version as two special cases that are key to the internals
of a running Accumulo. I don't see why we rely a consolidated Accumulo.getAccumuloPersistentVersion
and yet don't do the same with an Accumulo.getAccumuloInstanceId.


- Sean


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


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