accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ke...@deenlo.com
Subject Re: Review Request 17863: ACCUMULO-1832 add operation replace volumes
Date Sat, 08 Feb 2014 01:53:08 GMT


> On Feb. 8, 2014, 12:41 a.m., Josh Elser wrote:
> > core/src/main/java/org/apache/accumulo/core/conf/Property.java, line 132
> > <https://reviews.apache.org/r/17863/diff/1/?file=480463#file480463line132>
> >
> >     The syntax for these seem a little weird, but since you can specify a comma
in an HDFS path, that makes things more difficult.
> >     
> >     If you make the assertion that the URI can't contains quotation mark (because
you can use single or double quotes in a path in HDFS), you could make a slightly cleaner
parser: a comma-separated list of "URI_orig"|"URI_repl". Just a thought.
> 
> kturner wrote:
>     Based this decision on RFC3986[1].  AFAICT "|" is not a valid character anywhere
in a URI, it would have to be escaped.  
>     
>     [1]:http://www.ietf.org/rfc/rfc3986.txt
> 
> Josh Elser wrote:
>     Don't the volume URIs here potentially contain a path too? (e.g. hdfs://nn1/foo/bar)
>     
>     If so, HDFS isn't conforming to that RFC (try `hdfs dfs -mkdir "/fo|o"` for yourself)
and this isn't guaranteed to work.

Good point.  We can use standard URI escaping mechanism.   Would need to explicitly pass the
data through URI, the hadoop Path object does not seem to respect escaping.  Could update
the documentation to mention this.

This following code (7C is hex encoding of bar char) :

    System.out.println(new Path("hdfs://nn1/v1/a%7Cb"));
    System.out.println(new Path(new URI("hdfs://nn1/v1/a%7Cb")));

Prints :

    hdfs://nn1/v1/a%7Cb
    hdfs://nn1/v1/a|b


- kturner


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


On Feb. 7, 2014, 11:16 p.m., kturner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17863/
> -----------------------------------------------------------
> 
> (Updated Feb. 7, 2014, 11:16 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-1832
>     https://issues.apache.org/jira/browse/ACCUMULO-1832
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Add operation to replace volumes
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/conf/Property.java bc4c7e1 
>   core/src/main/java/org/apache/accumulo/core/tabletserver/log/LogEntry.java fc2da4b

>   server/base/src/main/java/org/apache/accumulo/server/ServerConstants.java 9d490e4 
>   server/base/src/main/java/org/apache/accumulo/server/fs/VolumeManager.java c2c04e5

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

>   server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java PRE-CREATION

>   server/base/src/main/java/org/apache/accumulo/server/master/recovery/HadoopLogCloser.java
0006bf9 
>   server/base/src/main/java/org/apache/accumulo/server/util/Admin.java 1e1bc79 
>   server/base/src/main/java/org/apache/accumulo/server/util/ListVolumesUsed.java PRE-CREATION

>   server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java 8b8a3d6

>   server/base/src/test/java/org/apache/accumulo/server/fs/FileTypeTest.java PRE-CREATION

>   server/base/src/test/java/org/apache/accumulo/server/fs/VolumeUtilTest.java PRE-CREATION

>   server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java 03519ba

>   server/master/src/main/java/org/apache/accumulo/master/recovery/RecoveryManager.java
2479d63 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/DirectoryDecommissioner.java
ea932ff 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/Tablet.java dd2afad 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java 080cc20

>   server/tserver/src/test/java/org/apache/accumulo/tserver/DirectoryDecommissionerTest.java
47cdab9 
>   server/tserver/src/test/java/org/apache/accumulo/tserver/TabletServerSyncCheckTest.java
590945a 
>   test/src/test/java/org/apache/accumulo/test/CleanWalIT.java ed04f7e 
>   test/src/test/java/org/apache/accumulo/test/ConfigurableMajorCompactionIT.java 28d183c

>   test/src/test/java/org/apache/accumulo/test/DumpConfigIT.java b98d452 
>   test/src/test/java/org/apache/accumulo/test/MasterRepairsDualAssignmentIT.java afd1403

>   test/src/test/java/org/apache/accumulo/test/NoMutationRecoveryIT.java 4656d30 
>   test/src/test/java/org/apache/accumulo/test/VolumeIT.java ee38efd 
>   test/src/test/java/org/apache/accumulo/test/functional/BalanceAfterCommsFailureIT.java
08242ff 
>   test/src/test/java/org/apache/accumulo/test/functional/BatchScanSplitIT.java faf8777

>   test/src/test/java/org/apache/accumulo/test/functional/BigRootTabletIT.java 16130bf

>   test/src/test/java/org/apache/accumulo/test/functional/BinaryStressIT.java 4a10a34

>   test/src/test/java/org/apache/accumulo/test/functional/BloomFilterIT.java e3914a1 
>   test/src/test/java/org/apache/accumulo/test/functional/BulkSplitOptimizationIT.java
337bd4f 
>   test/src/test/java/org/apache/accumulo/test/functional/ChaoticBalancerIT.java 9a4a904

>   test/src/test/java/org/apache/accumulo/test/functional/CleanTmpIT.java df1b51b 
>   test/src/test/java/org/apache/accumulo/test/functional/CompactionIT.java 3f8fcb3 
>   test/src/test/java/org/apache/accumulo/test/functional/ConcurrencyIT.java c09216f 
>   test/src/test/java/org/apache/accumulo/test/functional/ConfigurableCompactionIT.java
cc43cc9 
>   test/src/test/java/org/apache/accumulo/test/functional/ConfigurableMacIT.java 0de01c9

>   test/src/test/java/org/apache/accumulo/test/functional/DeleteEverythingIT.java f00a445

>   test/src/test/java/org/apache/accumulo/test/functional/DynamicThreadPoolsIT.java e83baee

>   test/src/test/java/org/apache/accumulo/test/functional/GarbageCollectorIT.java 81377bc

>   test/src/test/java/org/apache/accumulo/test/functional/HalfDeadTServerIT.java d8f38c8

>   test/src/test/java/org/apache/accumulo/test/functional/LargeRowIT.java c311413 
>   test/src/test/java/org/apache/accumulo/test/functional/LateLastContactIT.java 7f1f29b

>   test/src/test/java/org/apache/accumulo/test/functional/MasterFailoverIT.java 1fbe2ff

>   test/src/test/java/org/apache/accumulo/test/functional/MaxOpenIT.java 9483ab9 
>   test/src/test/java/org/apache/accumulo/test/functional/MetadataMaxFiles.java 789d84b

>   test/src/test/java/org/apache/accumulo/test/functional/MetadataSplitIT.java bb0dd15

>   test/src/test/java/org/apache/accumulo/test/functional/RestartIT.java 709aa1d 
>   test/src/test/java/org/apache/accumulo/test/functional/RestartStressIT.java da5a86f

>   test/src/test/java/org/apache/accumulo/test/functional/RowDeleteIT.java 0ee903b 
>   test/src/test/java/org/apache/accumulo/test/functional/ScanSessionTimeOutIT.java b223845

>   test/src/test/java/org/apache/accumulo/test/functional/SimpleBalancerFairnessIT.java
fcb19fd 
>   test/src/test/java/org/apache/accumulo/test/functional/SplitIT.java 80b79ff 
>   test/src/test/java/org/apache/accumulo/test/functional/SslIT.java 447f8ef 
>   test/src/test/java/org/apache/accumulo/test/functional/SslWithClientAuthIT.java 79b3255

>   test/src/test/java/org/apache/accumulo/test/functional/TabletIT.java d86742c 
>   test/src/test/java/org/apache/accumulo/test/functional/WriteAheadLogIT.java 25d0757

>   test/src/test/java/org/apache/accumulo/test/functional/ZookeeperRestartIT.java d944778

> 
> Diff: https://reviews.apache.org/r/17863/diff/
> 
> 
> Testing
> -------
> 
> Added unit test and IT.  VolumeIT is the only IT that needs review, all others were changed
because I changed a method signature
> 
> 
> Thanks,
> 
> kturner
> 
>


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