You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ozone.apache.org by sa...@apache.org on 2020/07/22 13:00:43 UTC

[hadoop-ozone] 33/39: HDDS-3982. Disable moveToTrash in o3fs and ofs temporarily (#1215)

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

sammichen pushed a commit to branch ozone-0.6.0
in repository https://gitbox.apache.org/repos/asf/hadoop-ozone.git

commit a90b7bba1599c9cd85081fb573dcb4fc64d1fada
Author: Siyao Meng <50...@users.noreply.github.com>
AuthorDate: Mon Jul 20 20:11:23 2020 -0700

    HDDS-3982. Disable moveToTrash in o3fs and ofs temporarily (#1215)
    
    (cherry picked from commit fbd125cd7cb1aea7941d251d31772ba3f37696b4)
---
 hadoop-hdds/docs/content/design/ofs.md             |  4 +++
 hadoop-hdds/docs/content/design/trash.md           |  7 +++-
 hadoop-hdds/docs/content/interface/OzoneFS.md      |  3 ++
 .../hadoop/fs/ozone/TestOzoneFileSystem.java       | 38 ++++++++++++++++++++++
 .../hadoop/fs/ozone/TestRootedOzoneFileSystem.java | 37 +++++++++++++++++++++
 .../hadoop/ozone/shell/TestOzoneShellHA.java       |  2 ++
 .../hadoop/fs/ozone/BasicOzoneFileSystem.java      | 29 +++++++++++++++++
 .../fs/ozone/BasicRootedOzoneFileSystem.java       | 29 +++++++++++++++++
 8 files changed, 148 insertions(+), 1 deletion(-)

diff --git a/hadoop-hdds/docs/content/design/ofs.md b/hadoop-hdds/docs/content/design/ofs.md
index 9a2352e..7a39665 100644
--- a/hadoop-hdds/docs/content/design/ofs.md
+++ b/hadoop-hdds/docs/content/design/ofs.md
@@ -155,6 +155,10 @@ This feature wouldn't degrade server performance as the loop is on the client.
 Think it as a client is issuing multiple requests to the server to get all the
 information.
 
+# Special note
+
+Trash is disabled even if `fs.trash.interval` is set on purpose. (HDDS-3982)
+
 # Link
 
 Design doc is uploaded to the JIRA HDDS-2665:
diff --git a/hadoop-hdds/docs/content/design/trash.md b/hadoop-hdds/docs/content/design/trash.md
index 78e077a..b936aae 100644
--- a/hadoop-hdds/docs/content/design/trash.md
+++ b/hadoop-hdds/docs/content/design/trash.md
@@ -22,4 +22,9 @@ author: Matthew Sharp
 
 The design doc is uploaded to the JIRA: 
 
-https://issues.apache.org/jira/secure/attachment/12985273/Ozone_Trash_Feature.docx
\ No newline at end of file
+https://issues.apache.org/jira/secure/attachment/12985273/Ozone_Trash_Feature.docx
+
+## Special note
+
+Trash is disabled for both o3fs and ofs even if `fs.trash.interval` is set
+on purpose. (HDDS-3982)
diff --git a/hadoop-hdds/docs/content/interface/OzoneFS.md b/hadoop-hdds/docs/content/interface/OzoneFS.md
index 98bf2f9..7aebe4f 100644
--- a/hadoop-hdds/docs/content/interface/OzoneFS.md
+++ b/hadoop-hdds/docs/content/interface/OzoneFS.md
@@ -165,3 +165,6 @@ hdfs dfs -put /etc/hosts /volume1/bucket1/test
 
 For more usage, see: https://issues.apache.org/jira/secure/attachment/12987636/Design%20ofs%20v1.pdf
 
+## Special note
+
+Trash is disabled even if `fs.trash.interval` is set on purpose. (HDDS-3982)
diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java
index 700506a..ad1705a 100644
--- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java
+++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java
@@ -33,6 +33,7 @@ import org.apache.hadoop.fs.FileAlreadyExistsException;
 import org.apache.hadoop.fs.FileStatus;
 import org.apache.hadoop.fs.FileSystem;
 import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.Trash;
 import org.apache.hadoop.fs.contract.ContractTestUtils;
 import org.apache.hadoop.hdds.conf.OzoneConfiguration;
 import org.apache.hadoop.ozone.MiniOzoneCluster;
@@ -47,6 +48,7 @@ import org.apache.hadoop.test.GenericTestUtils;
 
 import org.apache.commons.io.IOUtils;
 
+import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.FS_TRASH_INTERVAL_KEY;
 import static org.apache.hadoop.fs.FileSystem.TRASH_PREFIX;
 import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_FS_ITERATE_BATCH_SIZE;
 import static org.apache.hadoop.ozone.OzoneConsts.OZONE_URI_DELIMITER;
@@ -86,6 +88,7 @@ public class TestOzoneFileSystem {
   private String volumeName;
   private String bucketName;
   private int rootItemCount;
+  private Trash trash;
 
   @Test(timeout = 300_000)
   public void testCreateFileShouldCheckExistenceOfDirWithSameName()
@@ -167,6 +170,8 @@ public class TestOzoneFileSystem {
     testOzoneFsServiceLoader();
     o3fs = (OzoneFileSystem) fs;
 
+    testRenameToTrashDisabled();
+
     testGetTrashRoots();
     testGetTrashRoot();
     testGetDirectoryModificationTime();
@@ -197,6 +202,7 @@ public class TestOzoneFileSystem {
   private void setupOzoneFileSystem()
       throws IOException, TimeoutException, InterruptedException {
     OzoneConfiguration conf = new OzoneConfiguration();
+    conf.setInt(FS_TRASH_INTERVAL_KEY, 1);
     cluster = MiniOzoneCluster.newBuilder(conf)
         .setNumDatanodes(3)
         .build();
@@ -215,6 +221,7 @@ public class TestOzoneFileSystem {
     // Set the number of keys to be processed during batch operate.
     conf.setInt(OZONE_FS_ITERATE_BATCH_SIZE, 5);
     fs = FileSystem.get(conf);
+    trash = new Trash(conf);
   }
 
   private void testOzoneFsServiceLoader() throws IOException {
@@ -617,4 +624,35 @@ public class TestOzoneFileSystem {
     // Clean up
     o3fs.delete(trashRoot, true);
   }
+
+  /**
+   * Check that no files are actually moved to trash since it is disabled by
+   * fs.rename(src, dst, options).
+   */
+  public void testRenameToTrashDisabled() throws IOException {
+    // Create a file
+    String testKeyName = "testKey1";
+    Path path = new Path(OZONE_URI_DELIMITER, testKeyName);
+    try (FSDataOutputStream stream = fs.create(path)) {
+      stream.write(1);
+    }
+
+    // Call moveToTrash. We can't call protected fs.rename() directly
+    trash.moveToTrash(path);
+
+    // Construct paths
+    String username = UserGroupInformation.getCurrentUser().getShortUserName();
+    Path trashRoot = new Path(OZONE_URI_DELIMITER, TRASH_PREFIX);
+    Path userTrash = new Path(trashRoot, username);
+    Path userTrashCurrent = new Path(userTrash, "Current");
+    Path trashPath = new Path(userTrashCurrent, testKeyName);
+
+    // Trash Current directory should still have been created.
+    Assert.assertTrue(o3fs.exists(userTrashCurrent));
+    // Check under trash, the key should be deleted instead
+    Assert.assertFalse(o3fs.exists(trashPath));
+
+    // Cleanup
+    o3fs.delete(trashRoot, true);
+  }
 }
diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java
index 8c71c61..3aec3e8 100644
--- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java
+++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java
@@ -26,6 +26,7 @@ import org.apache.hadoop.fs.FileStatus;
 import org.apache.hadoop.fs.FileSystem;
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.fs.PathIsNotEmptyDirectoryException;
+import org.apache.hadoop.fs.Trash;
 import org.apache.hadoop.fs.contract.ContractTestUtils;
 import org.apache.hadoop.fs.permission.FsPermission;
 import org.apache.hadoop.hdds.conf.OzoneConfiguration;
@@ -64,6 +65,7 @@ import java.util.Set;
 import java.util.TreeSet;
 import java.util.stream.Collectors;
 
+import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.FS_TRASH_INTERVAL_KEY;
 import static org.apache.hadoop.fs.FileSystem.TRASH_PREFIX;
 import static org.apache.hadoop.fs.ozone.Constants.LISTING_PAGE_SIZE;
 import static org.apache.hadoop.ozone.OzoneAcl.AclScope.ACCESS;
@@ -87,6 +89,7 @@ public class TestRootedOzoneFileSystem {
   private RootedOzoneFileSystem ofs;
   private ObjectStore objectStore;
   private static BasicRootedOzoneClientAdapterImpl adapter;
+  private Trash trash;
 
   private String volumeName;
   private Path volumePath;
@@ -98,6 +101,7 @@ public class TestRootedOzoneFileSystem {
   @Before
   public void init() throws Exception {
     conf = new OzoneConfiguration();
+    conf.setInt(FS_TRASH_INTERVAL_KEY, 1);
     cluster = MiniOzoneCluster.newBuilder(conf)
         .setNumDatanodes(3)
         .build();
@@ -122,6 +126,7 @@ public class TestRootedOzoneFileSystem {
     //  hence this workaround.
     conf.set("fs.ofs.impl", "org.apache.hadoop.fs.ozone.RootedOzoneFileSystem");
     fs = FileSystem.get(conf);
+    trash = new Trash(conf);
     ofs = (RootedOzoneFileSystem) fs;
     adapter = (BasicRootedOzoneClientAdapterImpl) ofs.getAdapter();
   }
@@ -999,4 +1004,36 @@ public class TestRootedOzoneFileSystem {
     Assert.assertTrue(volume1.setOwner(prevOwner));
   }
 
+  /**
+   * Check that no files are actually moved to trash since it is disabled by
+   * fs.rename(src, dst, options).
+   */
+  @Test
+  public void testRenameToTrashDisabled() throws IOException {
+    // Create a file
+    String testKeyName = "testKey1";
+    Path path = new Path(bucketPath, testKeyName);
+    try (FSDataOutputStream stream = fs.create(path)) {
+      stream.write(1);
+    }
+
+    // Call moveToTrash. We can't call protected fs.rename() directly
+    trash.moveToTrash(path);
+
+    // Construct paths
+    String username = UserGroupInformation.getCurrentUser().getShortUserName();
+    Path trashRoot = new Path(bucketPath, TRASH_PREFIX);
+    Path userTrash = new Path(trashRoot, username);
+    Path userTrashCurrent = new Path(userTrash, "Current");
+    Path trashPath = new Path(userTrashCurrent, testKeyName);
+
+    // Trash Current directory should still have been created.
+    Assert.assertTrue(ofs.exists(userTrashCurrent));
+    // Check under trash, the key should be deleted instead
+    Assert.assertFalse(ofs.exists(trashPath));
+
+    // Cleanup
+    ofs.delete(trashRoot, true);
+  }
+
 }
diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestOzoneShellHA.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestOzoneShellHA.java
index e340d40..17baa06 100644
--- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestOzoneShellHA.java
+++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestOzoneShellHA.java
@@ -38,6 +38,7 @@ import org.junit.AfterClass;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.BeforeClass;
+import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.Timeout;
@@ -468,6 +469,7 @@ public class TestOzoneShellHA {
   }
 
   @Test
+  @Ignore("HDDS-3982. Disable moveToTrash in o3fs and ofs temporarily")
   public void testDeleteToTrashOrSkipTrash() throws Exception {
     final String hostPrefix = OZONE_OFS_URI_SCHEME + "://" + omServiceId;
     OzoneConfiguration clientConf = getClientConfForOFS(hostPrefix, conf);
diff --git a/hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicOzoneFileSystem.java b/hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicOzoneFileSystem.java
index 25dea3d..778277f 100644
--- a/hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicOzoneFileSystem.java
+++ b/hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicOzoneFileSystem.java
@@ -29,6 +29,7 @@ import org.apache.hadoop.fs.FileAlreadyExistsException;
 import org.apache.hadoop.fs.FileStatus;
 import org.apache.hadoop.fs.FileSystem;
 import org.apache.hadoop.fs.LocatedFileStatus;
+import org.apache.hadoop.fs.Options.Rename;
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.fs.PathIsNotEmptyDirectoryException;
 import org.apache.hadoop.fs.permission.FsPermission;
@@ -397,6 +398,34 @@ public class BasicOzoneFileSystem extends FileSystem {
     return result;
   }
 
+  /**
+   * Intercept rename to trash calls from TrashPolicyDefault,
+   * convert them to delete calls instead.
+   */
+  @Deprecated
+  protected void rename(final Path src, final Path dst,
+      final Rename... options) throws IOException {
+    boolean hasMoveToTrash = false;
+    if (options != null) {
+      for (Rename option : options) {
+        if (option == Rename.TO_TRASH) {
+          hasMoveToTrash = true;
+          break;
+        }
+      }
+    }
+    if (!hasMoveToTrash) {
+      // if doesn't have TO_TRASH option, just pass the call to super
+      super.rename(src, dst, options);
+    } else {
+      // intercept when TO_TRASH is found
+      LOG.info("Move to trash is disabled for o3fs, deleting instead: {}. "
+          + "Files or directories will NOT be retained in trash. "
+          + "Ignore the following TrashPolicyDefault message, if any.", src);
+      delete(src, true);
+    }
+  }
+
   private class DeleteIterator extends OzoneListingIterator {
     private boolean recursive;
 
diff --git a/hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java b/hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java
index 7c28428..3bea2bb 100644
--- a/hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java
+++ b/hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java
@@ -29,6 +29,7 @@ import org.apache.hadoop.fs.FileAlreadyExistsException;
 import org.apache.hadoop.fs.FileStatus;
 import org.apache.hadoop.fs.FileSystem;
 import org.apache.hadoop.fs.LocatedFileStatus;
+import org.apache.hadoop.fs.Options;
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.fs.PathIsNotEmptyDirectoryException;
 import org.apache.hadoop.fs.permission.FsPermission;
@@ -372,6 +373,34 @@ public class BasicRootedOzoneFileSystem extends FileSystem {
     return result;
   }
 
+  /**
+   * Intercept rename to trash calls from TrashPolicyDefault,
+   * convert them to delete calls instead.
+   */
+  @Deprecated
+  protected void rename(final Path src, final Path dst,
+      final Options.Rename... options) throws IOException {
+    boolean hasMoveToTrash = false;
+    if (options != null) {
+      for (Options.Rename option : options) {
+        if (option == Options.Rename.TO_TRASH) {
+          hasMoveToTrash = true;
+          break;
+        }
+      }
+    }
+    if (!hasMoveToTrash) {
+      // if doesn't have TO_TRASH option, just pass the call to super
+      super.rename(src, dst, options);
+    } else {
+      // intercept when TO_TRASH is found
+      LOG.info("Move to trash is disabled for ofs, deleting instead: {}. "
+          + "Files or directories will NOT be retained in trash. "
+          + "Ignore the following TrashPolicyDefault message, if any.", src);
+      delete(src, true);
+    }
+  }
+
   private class DeleteIterator extends OzoneListingIterator {
     final private boolean recursive;
     private final OzoneBucket bucket;


---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-commits-help@hadoop.apache.org