You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ozone.apache.org by ms...@apache.org on 2020/11/11 12:19:27 UTC

[ozone] branch master updated: HDDS-4307.Start Trash Emptier in Ozone Manager (#1507)

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

msingh pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git


The following commit(s) were added to refs/heads/master by this push:
     new 90190f5  HDDS-4307.Start Trash Emptier in Ozone Manager (#1507)
90190f5 is described below

commit 90190f50f42470ea25c68ee3db42e7294e8b44eb
Author: Sadanand Shenoy <sa...@gmail.com>
AuthorDate: Wed Nov 11 17:49:09 2020 +0530

    HDDS-4307.Start Trash Emptier in Ozone Manager (#1507)
---
 .../hadoop/fs/ozone/TestOzoneFileSystem.java       | 13 +++--
 .../hadoop/fs/ozone/TestRootedOzoneFileSystem.java | 54 ++++++++++++++++---
 .../org/apache/hadoop/ozone/om/OzoneManager.java   | 60 +++++++++++++++++++++-
 .../hadoop/fs/ozone/BasicOzoneFileSystem.java      |  9 +---
 .../fs/ozone/BasicRootedOzoneFileSystem.java       |  9 +---
 5 files changed, 116 insertions(+), 29 deletions(-)

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 46c0115..bd8d45c 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
@@ -218,7 +218,7 @@ public class TestOzoneFileSystem {
     testCreateWithInvalidPaths();
     testListStatusWithIntermediateDir();
 
-    testRenameToTrashDisabled();
+    testRenameToTrashEnabled();
 
     testGetTrashRoots();
     testGetTrashRoot();
@@ -748,10 +748,10 @@ public class TestOzoneFileSystem {
   }
 
   /**
-   * Check that no files are actually moved to trash since it is disabled by
-   * fs.rename(src, dst, options).
+   * Check that files are moved to trash.
+   * since fs.rename(src,dst,options) is enabled.
    */
-  public void testRenameToTrashDisabled() throws IOException {
+  public void testRenameToTrashEnabled() throws Exception {
     // Create a file
     String testKeyName = "testKey1";
     Path path = new Path(OZONE_URI_DELIMITER, testKeyName);
@@ -771,9 +771,8 @@ public class TestOzoneFileSystem {
 
     // 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));
-
+    // Check under trash, the key should be present
+    Assert.assertTrue(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 36cab1f..f8b07ab 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
@@ -70,6 +70,7 @@ 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.LOG;
 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;
@@ -1102,13 +1103,13 @@ public class TestRootedOzoneFileSystem {
   }
 
   /**
-   * Check that no files are actually moved to trash since it is disabled by
+   * Check that  files are moved to trash since it is enabled by
    * fs.rename(src, dst, options).
    */
   @Test
-  public void testRenameToTrashDisabled() throws IOException {
+  public void testRenameToTrashEnabled() throws IOException {
     // Create a file
-    String testKeyName = "testKey1";
+    String testKeyName = "testKey2";
     Path path = new Path(bucketPath, testKeyName);
     try (FSDataOutputStream stream = fs.create(path)) {
       stream.write(1);
@@ -1122,12 +1123,12 @@ public class TestRootedOzoneFileSystem {
     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);
-
+    String key = path.toString().substring(1);
+    Path trashPath = new Path(userTrashCurrent, key);
     // 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));
+    // Check under trash, the key should be present
+    Assert.assertTrue(ofs.exists(trashPath));
 
     // Cleanup
     ofs.delete(trashRoot, true);
@@ -1170,4 +1171,43 @@ public class TestRootedOzoneFileSystem {
     Boolean falseResult = fs.delete(parent, true);
     assertFalse(falseResult);
   }
+
+  /**
+   * 1.Move a Key to Trash
+   * 2.Verify that the key gets deleted by the trash emptier.
+   * @throws Exception
+   */
+  @Test
+  public void testTrash() throws Exception {
+    String testKeyName = "keyToBeDeleted";
+    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");
+    String key = path.toString().substring(1);
+    Path trashPath = new Path(userTrashCurrent, key);
+
+    // Wait until the TrashEmptier purges the key
+    GenericTestUtils.waitFor(()-> {
+      try {
+        return !ofs.exists(trashPath);
+      } catch (IOException e) {
+        LOG.error("Delete from Trash Failed");
+        Assert.fail("Delete from Trash Failed");
+        return false;
+      }
+    }, 1000, 180000);
+
+    // Cleanup
+    ofs.delete(trashRoot, true);
+
+  }
 }
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
index c2497c7..61709a5 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
@@ -30,6 +30,7 @@ import java.nio.file.Path;
 import java.nio.file.StandardCopyOption;
 import java.security.KeyPair;
 import java.security.PrivateKey;
+import java.security.PrivilegedExceptionAction;
 import java.security.PublicKey;
 import java.security.cert.CertificateException;
 import java.util.ArrayList;
@@ -48,11 +49,14 @@ import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.TimeUnit;
 
 import com.google.common.base.Optional;
+import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.conf.StorageUnit;
 import org.apache.hadoop.crypto.key.KeyProvider;
 import org.apache.hadoop.crypto.key.KeyProviderCryptoExtension;
 import org.apache.hadoop.fs.CommonConfigurationKeys;
 import org.apache.hadoop.fs.CommonConfigurationKeysPublic;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Trash;
 import org.apache.hadoop.hdds.HddsConfigKeys;
 import org.apache.hadoop.hdds.HddsUtils;
 import org.apache.hadoop.hdds.annotation.InterfaceAudience;
@@ -185,6 +189,8 @@ import com.google.protobuf.ProtocolMessageEnum;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.tuple.Pair;
 
+import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.FS_TRASH_INTERVAL_DEFAULT;
+import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.FS_TRASH_INTERVAL_KEY;
 import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_BLOCK_TOKEN_ENABLED;
 import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_BLOCK_TOKEN_ENABLED_DEFAULT;
 import static org.apache.hadoop.hdds.HddsUtils.getScmAddressForBlockClients;
@@ -335,6 +341,8 @@ public final class OzoneManager extends ServiceRuntimeInfoImpl
   // Used in MiniOzoneCluster testing
   private State omState;
 
+  private Thread emptier;
+
   private OzoneManager(OzoneConfiguration conf) throws IOException,
       AuthenticationException {
     super(OzoneVersionInfo.OZONE_VERSION_INFO);
@@ -1176,6 +1184,9 @@ public final class OzoneManager extends ServiceRuntimeInfoImpl
     }
     omRpcServer.start();
     isOmRpcServerRunning = true;
+    // TODO: Start this thread only on the leader node.
+    //  Should be fixed after HDDS-4451.
+    startTrashEmptier(configuration);
 
     registerMXBean();
 
@@ -1230,10 +1241,12 @@ public final class OzoneManager extends ServiceRuntimeInfoImpl
       // Allow OM to start as Http Server failure is not fatal.
       LOG.error("OM HttpServer failed to start.", ex);
     }
-
     omRpcServer.start();
     isOmRpcServerRunning = true;
 
+    // TODO: Start this thread only on the leader node.
+    //  Should be fixed after HDDS-4451.
+    startTrashEmptier(configuration);
     registerMXBean();
 
     startJVMPauseMonitor();
@@ -1241,6 +1254,45 @@ public final class OzoneManager extends ServiceRuntimeInfoImpl
     omState = State.RUNNING;
   }
 
+
+  /**
+   * Starts a Trash Emptier thread that does an fs.trashRoots and performs
+   * checkpointing & deletion.
+   * @param conf
+   * @throws IOException
+   */
+  private void startTrashEmptier(Configuration conf) throws IOException {
+    long trashInterval =
+            conf.getLong(FS_TRASH_INTERVAL_KEY, FS_TRASH_INTERVAL_DEFAULT);
+    if (trashInterval == 0) {
+      LOG.info("Trash Interval set to 0. Files deleted will not move to trash");
+      return;
+    } else if (trashInterval < 0) {
+      throw new IOException("Cannot start trash emptier with negative interval."
+              + " Set " + FS_TRASH_INTERVAL_KEY + " to a positive value.");
+    }
+
+    // configuration for the FS instance that  points to a root OFS uri.
+    // This will ensure that it will cover all volumes and buckets
+    Configuration fsconf = new Configuration();
+    String rootPath = String.format("%s://%s/",
+            OzoneConsts.OZONE_OFS_URI_SCHEME, conf.get(OZONE_OM_ADDRESS_KEY));
+
+    fsconf.set(CommonConfigurationKeysPublic.FS_DEFAULT_NAME_KEY, rootPath);
+    FileSystem fs = SecurityUtil.doAsLoginUser(
+            new PrivilegedExceptionAction<FileSystem>() {
+          @Override
+          public FileSystem run() throws IOException {
+            return FileSystem.get(fsconf);
+          }
+        });
+
+    this.emptier = new Thread(new Trash(fs, conf).
+      getEmptier(), "Trash Emptier");
+    this.emptier.setDaemon(true);
+    this.emptier.start();
+  }
+
   /**
    * Creates a new instance of rpc server. If an earlier instance is already
    * running then returns the same.
@@ -1322,6 +1374,12 @@ public final class OzoneManager extends ServiceRuntimeInfoImpl
       if (httpServer != null) {
         httpServer.stop();
       }
+      // TODO:Also stop this thread if an OM switches from leader to follower.
+      //  Should be fixed after HDDS-4451.
+      if (this.emptier != null) {
+        emptier.interrupt();
+        emptier = null;
+      }
       metadataManager.stop();
       metrics.unRegister();
       omClientProtocolMetrics.unregister();
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 8e976de..f6dedc8 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
@@ -407,8 +407,7 @@ public class BasicOzoneFileSystem extends FileSystem {
   }
 
   /**
-   * Intercept rename to trash calls from TrashPolicyDefault,
-   * convert them to delete calls instead.
+   * Intercept rename to trash calls from TrashPolicyDefault.
    */
   @Deprecated
   protected void rename(final Path src, final Path dst,
@@ -426,11 +425,7 @@ public class BasicOzoneFileSystem extends FileSystem {
       // 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);
+      rename(src, dst);
     }
   }
 
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 b1ea6f2..c8d6139 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
@@ -385,8 +385,7 @@ public class BasicRootedOzoneFileSystem extends FileSystem {
   }
 
   /**
-   * Intercept rename to trash calls from TrashPolicyDefault,
-   * convert them to delete calls instead.
+   * Intercept rename to trash calls from TrashPolicyDefault.
    */
   @Deprecated
   protected void rename(final Path src, final Path dst,
@@ -404,11 +403,7 @@ public class BasicRootedOzoneFileSystem extends FileSystem {
       // 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);
+      rename(src, dst);
     }
   }
 


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