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 19142: Deprecate instance.dfs.dir and instance.dfs.uri
Date Sun, 16 Mar 2014 03:39:48 GMT


> On March 13, 2014, 7:32 p.m., kturner wrote:
> > server/base/src/main/java/org/apache/accumulo/server/fs/VolumeManagerImpl.java,
line 69
> > <https://reviews.apache.org/r/19142/diff/2/?file=517719#file517719line69>
> >
> >     Filesystem is an abstract class.  I do not think using it as a Key is good.
 Do all implementation of FileSystem have good implementations of equals() and hashCode()?

Good point -- I can't imagine that all implementations would have unique hashCodes across
one another. Maybe using the URI for the FileSystem is a better idea? (e.g. hdfs://nn:port,
file://, s3://bucket) I'm guessing that the scheme should help a bit.


> On March 13, 2014, 7:32 p.m., kturner wrote:
> > server/base/src/main/java/org/apache/accumulo/server/fs/VolumeManagerImpl.java,
line 315
> > <https://reviews.apache.org/r/19142/diff/2/?file=517719#file517719line315>
> >
> >     Tablets can point to files on volumes that were configured in the past, but
are no longer configured.  For this case the correct filesystem needs to be returned, not
the default filesystem.  The volumes currently configured are what should be used to create
new files, but code should not assume those are the only volumes in use.

Noted. It's easy enough to fix that, but, without a proper definition in instance.volumes
for that FileSystem, I can't create a "correct" Volume object (because we don't know what
the base path within that Volume should be). I think this is OK because, like you said, we'll
only read from that Volume and not try to create new files on that Volume.

I'll write up a new test for this as well -- should be easy enough to replicate.


- Josh


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


On March 12, 2014, 8:51 p.m., Josh Elser wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19142/
> -----------------------------------------------------------
> 
> (Updated March 12, 2014, 8:51 p.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/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 
>   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