This is an automated email from the ASF dual-hosted git repository.

ferhui pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/hadoop.git


The following commit(s) were added to refs/heads/trunk by this push:
     new a4f4590  HADOOP-18117. Add an option to preserve root directory 
permissions (#3970)
a4f4590 is described below

commit a4f459097b068516702a1edfc1b72384f6fd37ee
Author: Mohanad Elsafty <[email protected]>
AuthorDate: Fri Feb 18 19:12:50 2022 +0800

    HADOOP-18117. Add an option to preserve root directory permissions (#3970)
---
 .../org/apache/hadoop/tools/DistCpConstants.java   |  2 +
 .../org/apache/hadoop/tools/DistCpContext.java     |  4 ++
 .../apache/hadoop/tools/DistCpOptionSwitch.java    |  6 ++-
 .../org/apache/hadoop/tools/DistCpOptions.java     | 19 ++++++++++
 .../org/apache/hadoop/tools/OptionsParser.java     |  4 +-
 .../org/apache/hadoop/tools/SimpleCopyListing.java |  6 ++-
 .../apache/hadoop/tools/mapred/CopyCommitter.java  | 10 ++++-
 .../hadoop-distcp/src/site/markdown/DistCp.md.vm   |  1 +
 .../org/apache/hadoop/tools/TestDistCpOptions.java | 13 ++++++-
 .../org/apache/hadoop/tools/TestDistCpSystem.java  | 43 ++++++++++++++++++++++
 .../org/apache/hadoop/tools/TestOptionsParser.java | 14 +++++++
 11 files changed, 115 insertions(+), 7 deletions(-)

diff --git 
a/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/DistCpConstants.java
 
b/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/DistCpConstants.java
index c75c0e8..289d552 100644
--- 
a/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/DistCpConstants.java
+++ 
b/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/DistCpConstants.java
@@ -86,6 +86,8 @@ public final class DistCpConstants {
   public static final String CONF_LABEL_SPLIT_RATIO =
       "distcp.dynamic.split.ratio";
   public static final String CONF_LABEL_DIRECT_WRITE = "distcp.direct.write";
+  public static final String CONF_LABEL_UPDATE_ROOT =
+          "distcp.update.root.attributes";
 
   /* Total bytes to be copied. Updated by copylisting. Unfiltered count */
   public static final String CONF_LABEL_TOTAL_BYTES_TO_BE_COPIED = 
"mapred.total.bytes.expected";
diff --git 
a/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/DistCpContext.java
 
b/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/DistCpContext.java
index 8e9d64a..8c443f6 100644
--- 
a/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/DistCpContext.java
+++ 
b/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/DistCpContext.java
@@ -175,6 +175,10 @@ public class DistCpContext {
     return options.shouldUseIterator();
   }
 
+  public boolean shouldUpdateRoot() {
+    return options.shouldUpdateRoot();
+  }
+
   public final boolean splitLargeFile() {
     return options.getBlocksPerChunk() > 0;
   }
diff --git 
a/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/DistCpOptionSwitch.java
 
b/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/DistCpOptionSwitch.java
index 4163f82..becd537 100644
--- 
a/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/DistCpOptionSwitch.java
+++ 
b/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/DistCpOptionSwitch.java
@@ -244,8 +244,12 @@ public enum DistCpOptionSwitch {
   USE_ITERATOR(DistCpConstants.CONF_LABEL_USE_ITERATOR,
       new Option("useiterator", false,
           "Use single threaded list status iterator to build "
-              + "the listing to save the memory utilisation at the client"));
+              + "the listing to save the memory utilisation at the client")),
 
+  UPDATE_ROOT(DistCpConstants.CONF_LABEL_UPDATE_ROOT,
+      new Option("updateRoot", false,
+      "Update root directory attributes "
+          + "(eg permissions, ownership ...)"));
 
   public static final String PRESERVE_STATUS_DEFAULT = "-prbugpct";
   private final String confLabel;
diff --git 
a/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/DistCpOptions.java
 
b/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/DistCpOptions.java
index fee53ec..e5685c1 100644
--- 
a/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/DistCpOptions.java
+++ 
b/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/DistCpOptions.java
@@ -162,6 +162,8 @@ public final class DistCpOptions {
 
   private final boolean useIterator;
 
+  private final boolean updateRoot;
+
   /**
    * File attributes for preserve.
    *
@@ -228,6 +230,8 @@ public final class DistCpOptions {
     this.directWrite = builder.directWrite;
 
     this.useIterator = builder.useIterator;
+
+    this.updateRoot = builder.updateRoot;
   }
 
   public Path getSourceFileListing() {
@@ -374,6 +378,10 @@ public final class DistCpOptions {
     return useIterator;
   }
 
+  public boolean shouldUpdateRoot() {
+    return updateRoot;
+  }
+
   /**
    * Add options to configuration. These will be used in the Mapper/committer
    *
@@ -427,6 +435,9 @@ public final class DistCpOptions {
 
     DistCpOptionSwitch.addToConf(conf, DistCpOptionSwitch.USE_ITERATOR,
         String.valueOf(useIterator));
+
+    DistCpOptionSwitch.addToConf(conf, DistCpOptionSwitch.UPDATE_ROOT,
+        String.valueOf(updateRoot));
   }
 
   /**
@@ -465,6 +476,7 @@ public final class DistCpOptions {
         ", verboseLog=" + verboseLog +
         ", directWrite=" + directWrite +
         ", useiterator=" + useIterator +
+        ", updateRoot=" + updateRoot +
         '}';
   }
 
@@ -518,6 +530,8 @@ public final class DistCpOptions {
 
     private boolean useIterator = false;
 
+    private boolean updateRoot = false;
+
     public Builder(List<Path> sourcePaths, Path targetPath) {
       Preconditions.checkArgument(sourcePaths != null && 
!sourcePaths.isEmpty(),
           "Source paths should not be null or empty!");
@@ -780,6 +794,11 @@ public final class DistCpOptions {
       this.useIterator = useItr;
       return this;
     }
+
+    public Builder withUpdateRoot(boolean updateRootAttrs) {
+      this.updateRoot = updateRootAttrs;
+      return this;
+    }
   }
 
 }
diff --git 
a/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/OptionsParser.java
 
b/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/OptionsParser.java
index 476b57b..f2875c5 100644
--- 
a/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/OptionsParser.java
+++ 
b/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/OptionsParser.java
@@ -117,7 +117,9 @@ public class OptionsParser {
         .withDirectWrite(
             command.hasOption(DistCpOptionSwitch.DIRECT_WRITE.getSwitch()))
         .withUseIterator(
-            command.hasOption(DistCpOptionSwitch.USE_ITERATOR.getSwitch()));
+            command.hasOption(DistCpOptionSwitch.USE_ITERATOR.getSwitch()))
+        .withUpdateRoot(
+            command.hasOption(DistCpOptionSwitch.UPDATE_ROOT.getSwitch()));
 
     if (command.hasOption(DistCpOptionSwitch.DIFF.getSwitch())) {
       String[] snapshots = getVals(command,
diff --git 
a/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/SimpleCopyListing.java
 
b/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/SimpleCopyListing.java
index e9eb4c0..c3f097d 100644
--- 
a/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/SimpleCopyListing.java
+++ 
b/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/SimpleCopyListing.java
@@ -616,10 +616,12 @@ public class SimpleCopyListing extends CopyListing {
       DistCpContext context) throws IOException {
     boolean syncOrOverwrite = context.shouldSyncFolder() ||
         context.shouldOverwrite();
+    boolean skipRootPath = syncOrOverwrite && !context.shouldUpdateRoot();
     for (CopyListingFileStatus fs : fileStatus) {
       if (fs.getPath().equals(sourcePathRoot) &&
-          fs.isDirectory() && syncOrOverwrite) {
-        // Skip the root-paths when syncOrOverwrite
+          fs.isDirectory() && skipRootPath) {
+        // Skip the root-paths when skipRootPath (syncOrOverwrite and
+        // update root directory is not a must).
         LOG.debug("Skip {}", fs.getPath());
         return;
       }
diff --git 
a/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/mapred/CopyCommitter.java
 
b/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/mapred/CopyCommitter.java
index 2272781..8c2bc82 100644
--- 
a/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/mapred/CopyCommitter.java
+++ 
b/hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/mapred/CopyCommitter.java
@@ -75,6 +75,7 @@ public class CopyCommitter extends FileOutputCommitter {
   private boolean ignoreFailures = false;
   private boolean skipCrc = false;
   private int blocksPerChunk = 0;
+  private boolean updateRoot = false;
 
   /**
    * Create a output committer
@@ -100,6 +101,8 @@ public class CopyCommitter extends FileOutputCommitter {
     Configuration conf = jobContext.getConfiguration();
     syncFolder = conf.getBoolean(DistCpConstants.CONF_LABEL_SYNC_FOLDERS, 
false);
     overwrite = conf.getBoolean(DistCpConstants.CONF_LABEL_OVERWRITE, false);
+    updateRoot =
+        conf.getBoolean(CONF_LABEL_UPDATE_ROOT, false);
     targetPathExists = conf.getBoolean(
         DistCpConstants.CONF_LABEL_TARGET_PATH_EXISTS, true);
     ignoreFailures = conf.getBoolean(
@@ -336,9 +339,12 @@ public class CopyCommitter extends FileOutputCommitter {
 
         Path targetFile = new Path(targetRoot.toString() + "/" + srcRelPath);
         //
-        // Skip the root folder when syncOrOverwrite is true.
+        // Skip the root folder when skipRoot is true.
         //
-        if (targetRoot.equals(targetFile) && syncOrOverwrite) continue;
+        boolean skipRoot = syncOrOverwrite && !updateRoot;
+        if (targetRoot.equals(targetFile) && skipRoot) {
+          continue;
+        }
 
         FileSystem targetFS = targetFile.getFileSystem(conf);
         DistCpUtils.preserve(targetFS, targetFile, srcFileStatus, attributes,
diff --git a/hadoop-tools/hadoop-distcp/src/site/markdown/DistCp.md.vm 
b/hadoop-tools/hadoop-distcp/src/site/markdown/DistCp.md.vm
index 136b6c8..030f20c 100644
--- a/hadoop-tools/hadoop-distcp/src/site/markdown/DistCp.md.vm
+++ b/hadoop-tools/hadoop-distcp/src/site/markdown/DistCp.md.vm
@@ -363,6 +363,7 @@ Command Line Options
 | `-xtrack <path>` | Save information about missing source files to the 
specified path. | This option is only valid with `-update` option. This is an 
experimental property and it cannot be used with `-atomic` option. |
 | `-direct` | Write directly to destination paths | Useful for avoiding 
potentially very expensive temporary file rename operations when the 
destination is an object store |
 | `-useiterator` | Uses single threaded listStatusIterator to build listing | 
Useful for saving memory at the client side. Using this option will ignore the 
numListstatusThreads option |
+| `-updateRoot` | Update root directory attributes (eg permissions, ownership 
...) | Useful if you need to enforce root directory attributes update when 
using distcp |
 
 Architecture of DistCp
 ----------------------
diff --git 
a/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/TestDistCpOptions.java
 
b/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/TestDistCpOptions.java
index 1349702..ac2100e 100644
--- 
a/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/TestDistCpOptions.java
+++ 
b/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/TestDistCpOptions.java
@@ -289,7 +289,7 @@ public class TestDistCpOptions {
         "atomicWorkPath=null, logPath=null, sourceFileListing=abc, " +
         "sourcePaths=null, targetPath=xyz, filtersFile='null', " +
         "blocksPerChunk=0, copyBufferSize=8192, verboseLog=false, " +
-        "directWrite=false, useiterator=false}";
+        "directWrite=false, useiterator=false, updateRoot=false}";
     String optionString = option.toString();
     Assert.assertEquals(val, optionString);
     Assert.assertNotSame(DistCpOptionSwitch.ATOMIC_COMMIT.toString(),
@@ -563,4 +563,15 @@ public class TestDistCpOptions {
             "otherwise it may not be fetched properly",
             expectedValForEmptyConfigKey, config.get(""));
   }
+
+  @Test
+  public void testUpdateRoot() {
+    final DistCpOptions options = new DistCpOptions.Builder(
+        Collections.singletonList(
+            new Path("hdfs://localhost:8020/source")),
+        new Path("hdfs://localhost:8020/target/"))
+        .withUpdateRoot(true)
+        .build();
+    Assert.assertTrue(options.shouldUpdateRoot());
+  }
 }
diff --git 
a/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/TestDistCpSystem.java
 
b/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/TestDistCpSystem.java
index 47b850f..64c6800 100644
--- 
a/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/TestDistCpSystem.java
+++ 
b/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/TestDistCpSystem.java
@@ -21,6 +21,7 @@ package org.apache.hadoop.tools;
 import static org.apache.hadoop.test.GenericTestUtils.getMethodName;
 import static org.hamcrest.core.Is.is;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertTrue;
 
 import java.io.ByteArrayOutputStream;
@@ -44,6 +45,8 @@ import org.apache.hadoop.hdfs.DFSTestUtil;
 import org.apache.hadoop.hdfs.DistributedFileSystem;
 import org.apache.hadoop.hdfs.MiniDFSCluster;
 import org.apache.hadoop.hdfs.client.HdfsClientConfigKeys;
+import org.apache.hadoop.test.GenericTestUtils;
+import org.apache.hadoop.tools.util.DistCpTestUtils;
 import org.apache.hadoop.util.ToolRunner;
 import org.junit.AfterClass;
 import org.junit.Assert;
@@ -551,4 +554,44 @@ public class TestDistCpSystem {
     String[] args2 = new String[]{rootStr, tgtStr2};
     Assert.assertThat(ToolRunner.run(conf, new DistCp(), args2), is(0));
   }
+
+  @Test
+  public void testUpdateRoot() throws Exception {
+    FileSystem fs = cluster.getFileSystem();
+
+    Path source = new Path("/src");
+    Path dest1 = new Path("/dest1");
+    Path dest2 = new Path("/dest2");
+
+    fs.delete(source, true);
+    fs.delete(dest1, true);
+    fs.delete(dest2, true);
+
+    // Create a source dir
+    fs.mkdirs(source);
+    fs.setOwner(source, "userA", "groupA");
+    fs.setTimes(source, new Random().nextLong(), new Random().nextLong());
+
+    GenericTestUtils.createFiles(fs, source, 3, 5, 5);
+
+    // should not preserve attrs
+    DistCpTestUtils.assertRunDistCp(DistCpConstants.SUCCESS, source.toString(),
+        dest1.toString(), "-p -update", conf);
+
+    FileStatus srcStatus = fs.getFileStatus(source);
+    FileStatus destStatus1 = fs.getFileStatus(dest1);
+    assertNotEquals(srcStatus.getOwner(), destStatus1.getOwner());
+    assertNotEquals(srcStatus.getModificationTime(),
+        destStatus1.getModificationTime());
+
+    // should preserve attrs
+    DistCpTestUtils.assertRunDistCp(DistCpConstants.SUCCESS, source.toString(),
+        dest2.toString(), "-p -update -updateRoot",
+        conf);
+
+    FileStatus destStatus2 = fs.getFileStatus(dest2);
+    assertEquals(srcStatus.getOwner(), destStatus2.getOwner());
+    assertEquals(srcStatus.getModificationTime(),
+        destStatus2.getModificationTime());
+  }
 }
\ No newline at end of file
diff --git 
a/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/TestOptionsParser.java
 
b/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/TestOptionsParser.java
index 85b312a..1ffdd89 100644
--- 
a/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/TestOptionsParser.java
+++ 
b/hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/TestOptionsParser.java
@@ -804,4 +804,18 @@ public class TestOptionsParser {
         "hdfs://localhost:8020/target/"});
     assertThat(options.getFiltersFile()).isEqualTo("/tmp/filters.txt");
   }
+
+  @Test
+  public void testParseUpdateRoot() {
+    DistCpOptions options = OptionsParser.parse(new String[] {
+        "hdfs://localhost:8020/source/first",
+        "hdfs://localhost:8020/target/"});
+    Assert.assertFalse(options.shouldUpdateRoot());
+
+    options = OptionsParser.parse(new String[] {
+        "-updateRoot",
+        "hdfs://localhost:8020/source/first",
+        "hdfs://localhost:8020/target/"});
+    Assert.assertTrue(options.shouldUpdateRoot());
+  }
 }

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to