You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by el...@apache.org on 2021/11/18 01:15:52 UTC

[hbase-filesystem] branch master updated (04a304e -> 21e1e92)

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

elserj pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/hbase-filesystem.git.


    from 04a304e  HBASE-26315 Upgrade commons-io to 2.11.0 in hbase-filesystem (#27)
     new 0aaacf6  HBASE-26453 Correct the behavior of isBeneath to not consider paths which share a name prefix as beneath one another.
     new 21e1e92  HBASE-26437 Clean up the znodes for the src after a rename.

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../hbase/oss/HBaseObjectStoreSemantics.java       |  20 ++-
 .../hadoop/hbase/oss/sync/TreeLockManager.java     |  26 ++-
 .../hadoop/hbase/oss/sync/ZKTreeLockManager.java   |  45 +++--
 .../hbase/oss/HBaseObjectStoreSemanticsTest.java   |   4 +
 .../apache/hadoop/hbase/oss/TestZNodeCleanup.java  | 188 +++++++++++++++++++++
 .../hadoop/hbase/oss/sync/TestTreeLockManager.java |   2 +-
 .../hbase/oss/sync/TestZKTreeLockManager.java}     |  34 ++--
 hbase-oss/src/test/resources/log4j.properties      |   2 +
 8 files changed, 289 insertions(+), 32 deletions(-)
 create mode 100644 hbase-oss/src/test/java/org/apache/hadoop/hbase/oss/TestZNodeCleanup.java
 copy hbase-oss/src/{main/java/org/apache/hadoop/hbase/oss/Constants.java => test/java/org/apache/hadoop/hbase/oss/sync/TestZKTreeLockManager.java} (51%)

[hbase-filesystem] 01/02: HBASE-26453 Correct the behavior of isBeneath to not consider paths which share a name prefix as beneath one another.

Posted by el...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

elserj pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hbase-filesystem.git

commit 0aaacf63cd784566a1f98555839e068ac6a0e8d0
Author: Josh Elser <el...@apache.org>
AuthorDate: Fri Nov 12 12:57:07 2021 -0500

    HBASE-26453 Correct the behavior of isBeneath to not consider paths which share a name prefix as beneath one another.
    
    The current implementation of isBeneath fails when given paths of the
    form '/foo' and '/foobar' (returning that '/foobar' is beneath '/foo').
    
    Because this method returns incorrect values, it causes Curator mutexes
    to be removed and znodes to be removed while they were potentially held.
    
    Signed-off-by: Wellington Chevreuil <wc...@apache.org>
---
 .../hadoop/hbase/oss/sync/ZKTreeLockManager.java   | 29 ++++++++++++---
 .../hbase/oss/sync/TestZKTreeLockManager.java      | 43 ++++++++++++++++++++++
 2 files changed, 67 insertions(+), 5 deletions(-)

diff --git a/hbase-oss/src/main/java/org/apache/hadoop/hbase/oss/sync/ZKTreeLockManager.java b/hbase-oss/src/main/java/org/apache/hadoop/hbase/oss/sync/ZKTreeLockManager.java
index 0b8f819..549d4c1 100644
--- a/hbase-oss/src/main/java/org/apache/hadoop/hbase/oss/sync/ZKTreeLockManager.java
+++ b/hbase-oss/src/main/java/org/apache/hadoop/hbase/oss/sync/ZKTreeLockManager.java
@@ -241,7 +241,7 @@ public class ZKTreeLockManager extends TreeLockManager {
     }
   }
 
-  private synchronized void removeInMemoryLocks(Path p) {
+  synchronized void removeInMemoryLocks(Path p) {
     Iterator<Entry<Path,InterProcessReadWriteLock>> iter = lockCache.entrySet().iterator();
     while (iter.hasNext()) {
       Entry<Path,InterProcessReadWriteLock> entry = iter.next();
@@ -252,12 +252,31 @@ public class ZKTreeLockManager extends TreeLockManager {
     }
   }
 
-  private boolean isBeneath(Path parent, Path other) {
-    if (parent.equals(other)) {
+  /**
+   * Returns true iff the given path is contained beneath the parent path.
+   *
+   * Specifically, this method will return true if the given path is a sub-directory
+   * of the parent or a file in the directory represented by the parent. This method
+   * returns false if the parent and the given path are the same. 
+   */
+  boolean isBeneath(Path parent, Path given) {
+    if (parent.equals(given)) {
+      return false;
+    }
+    String parentPathStr = parent.toString();
+    String givenPathStr = given.toString();
+    int offset = givenPathStr.indexOf(parentPathStr);
+    // Is the given path fully contained in some path beneath the parent.
+    if (0 != offset) {
       return false;
     }
-    // Is `other` fully contained in some path beneath the parent.
-    return 0 == other.toString().indexOf(parent.toString());
+    // The given path is a substring of the parent path. It might share a common name (e.g. /foo and /foo1)
+    // or it might be a subdirectory or file in the parent (e.g. /foo and /foo/bar)
+    String givenRemainer = givenPathStr.substring(parentPathStr.length());
+    // If the remainder of the given path starts with a '/', then it's contained beneath the parent.
+    // If there are additional characters, the given path simple shares a prefix in the file/dir represented
+    // by the parent.
+    return givenRemainer.startsWith(Path.SEPARATOR);
   }
 
   private boolean writeLockBelow(Path p, int level, int maxLevel) throws IOException {
diff --git a/hbase-oss/src/test/java/org/apache/hadoop/hbase/oss/sync/TestZKTreeLockManager.java b/hbase-oss/src/test/java/org/apache/hadoop/hbase/oss/sync/TestZKTreeLockManager.java
new file mode 100644
index 0000000..075f951
--- /dev/null
+++ b/hbase-oss/src/test/java/org/apache/hadoop/hbase/oss/sync/TestZKTreeLockManager.java
@@ -0,0 +1,43 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.oss.sync;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import org.apache.hadoop.fs.Path;
+import org.junit.Before;
+import org.junit.Test;
+
+public class TestZKTreeLockManager {
+  ZKTreeLockManager manager;
+
+  @Before
+  public void setup() {
+    this.manager = new ZKTreeLockManager();
+  }
+
+  @Test
+  public void testIsBeneath() {
+    assertFalse(manager.isBeneath(new Path("/foo"), new Path("/bar")));
+    assertTrue(manager.isBeneath(new Path("/foo"), new Path("/foo/bar")));
+    assertFalse(manager.isBeneath(new Path("/foo"), new Path("/foo2")));
+    assertTrue(manager.isBeneath(new Path("/foo/bar"), new Path("/foo/bar/2")));
+    assertFalse(manager.isBeneath(new Path("/foo"), new Path("/foo")));
+  }
+}

[hbase-filesystem] 02/02: HBASE-26437 Clean up the znodes for the src after a rename.

Posted by el...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

elserj pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hbase-filesystem.git

commit 21e1e920b7f15dbb1bfb8b261d39d303ea97f3e2
Author: Josh Elser <el...@apache.org>
AuthorDate: Wed Nov 10 13:13:32 2021 -0500

    HBASE-26437 Clean up the znodes for the src after a rename.
    
    HBOSS was orphaning znodes from the src of a path which is renamed. Over
    time, this will result in a very large usage of ZK due to HBOSS.
    
    Add some logging to dump the contents of ZK
    
    Signed-off-by: Wellington Chevreuil <wc...@apache.org>
    
    Closes #29
---
 .../hbase/oss/HBaseObjectStoreSemantics.java       |  20 ++-
 .../hadoop/hbase/oss/sync/TreeLockManager.java     |  26 ++-
 .../hadoop/hbase/oss/sync/ZKTreeLockManager.java   |  18 +-
 .../hbase/oss/HBaseObjectStoreSemanticsTest.java   |   4 +
 .../apache/hadoop/hbase/oss/TestZNodeCleanup.java  | 188 +++++++++++++++++++++
 .../hadoop/hbase/oss/sync/TestTreeLockManager.java |   2 +-
 hbase-oss/src/test/resources/log4j.properties      |   2 +
 7 files changed, 246 insertions(+), 14 deletions(-)

diff --git a/hbase-oss/src/main/java/org/apache/hadoop/hbase/oss/HBaseObjectStoreSemantics.java b/hbase-oss/src/main/java/org/apache/hadoop/hbase/oss/HBaseObjectStoreSemantics.java
index 7669181..65eeb16 100644
--- a/hbase-oss/src/main/java/org/apache/hadoop/hbase/oss/HBaseObjectStoreSemantics.java
+++ b/hbase-oss/src/main/java/org/apache/hadoop/hbase/oss/HBaseObjectStoreSemantics.java
@@ -24,6 +24,8 @@ import java.net.URI;
 import java.util.EnumSet;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.BlockLocation;
 import org.apache.hadoop.fs.ContentSummary;
@@ -452,13 +454,21 @@ public class HBaseObjectStoreSemantics extends FilterFileSystem {
     long startTime = System.currentTimeMillis();
     long lockAcquiredTime = startTime;
     long doneTime = startTime;
-    try (AutoLock l = sync.lockRename(src, dst)) {
+    // Future to pass into the AutoLock so it knows if it should clean up.
+    final CompletableFuture<Boolean> renameResult = new CompletableFuture<>();
+    try (AutoLock l = sync.lockRename(src, dst, renameResult)) {
       lockAcquiredTime = System.currentTimeMillis();
       metrics.updateAcquireRenameLockHisto(lockAcquiredTime- startTime);
-      boolean result = fs.rename(src, dst);
-      doneTime = System.currentTimeMillis();
-      metrics.updateRenameFsOperationHisto(doneTime - lockAcquiredTime);
-      return result;
+      // Defaulting to false in the case that fs.rename throws an exception
+      boolean result = false;
+      try {
+        result = fs.rename(src, dst);
+        return result;
+      } finally {
+        renameResult.complete(result);
+        doneTime = System.currentTimeMillis();
+        metrics.updateRenameFsOperationHisto(doneTime - lockAcquiredTime);
+      }
     }
     finally {
       long releasedLocksTime = System.currentTimeMillis();
diff --git a/hbase-oss/src/main/java/org/apache/hadoop/hbase/oss/sync/TreeLockManager.java b/hbase-oss/src/main/java/org/apache/hadoop/hbase/oss/sync/TreeLockManager.java
index 22209e4..9506262 100644
--- a/hbase-oss/src/main/java/org/apache/hadoop/hbase/oss/sync/TreeLockManager.java
+++ b/hbase-oss/src/main/java/org/apache/hadoop/hbase/oss/sync/TreeLockManager.java
@@ -21,7 +21,10 @@ package org.apache.hadoop.hbase.oss.sync;
 import java.io.IOException;
 import java.net.URI;
 import java.util.Arrays;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Future;
 import java.util.concurrent.TimeUnit;
+
 import org.apache.hadoop.io.retry.RetryPolicies;
 import org.apache.hadoop.io.retry.RetryPolicy;
 import org.apache.hadoop.io.retry.RetryPolicy.RetryAction;
@@ -471,7 +474,7 @@ public abstract class TreeLockManager {
    * @return AutoCloseable to release both paths
    * @throws IOException at any possible IO failure.
    */
-  public AutoLock lockRename(Path rawSrc, Path rawDst) throws IOException {
+  public AutoLock lockRename(Path rawSrc, Path rawDst, Future<Boolean> successFuture) throws IOException {
     Path src = norm(rawSrc);
     Path dst = norm(rawDst);
     LOG.debug("About to lock for rename: from {} to {}", src, dst);
@@ -484,8 +487,29 @@ public abstract class TreeLockManager {
     }
     return new AutoLock() {
       public void close() throws IOException {
+        // We have to clean up the src znodes:
+        //   1. If the rename was successful
+        //   2. While we still hold the write lock
         LOG.debug("About to unlock after rename: from {} to {}", src, dst);
         try {
+          Boolean renameSuccess;
+          try {
+            renameSuccess = successFuture.get();
+          } catch (InterruptedException | ExecutionException e) {
+            LOG.warn("Unable to determine if filesystem rename was successful. Assuming it failed.", e);
+            renameSuccess = false;
+          }
+          if (renameSuccess != null && renameSuccess.booleanValue()) {
+            // Tricky... HBossContract tests tough things like
+            //   `rename("/", "/somethingelse")`
+            // This means we grabbed write locks on
+            //    /               (src)
+            //    /somethingelse  (dst)
+            // Thus, we can't safely delete the znodes for src as it may
+            // then also affect the (held) lock on the dst. This is why
+            // we only delete the znodes on success.
+            recursiveDelete(src);
+          }
           writeUnlock(src);
         } finally {
           writeUnlock(dst);
diff --git a/hbase-oss/src/main/java/org/apache/hadoop/hbase/oss/sync/ZKTreeLockManager.java b/hbase-oss/src/main/java/org/apache/hadoop/hbase/oss/sync/ZKTreeLockManager.java
index 549d4c1..16bd57e 100644
--- a/hbase-oss/src/main/java/org/apache/hadoop/hbase/oss/sync/ZKTreeLockManager.java
+++ b/hbase-oss/src/main/java/org/apache/hadoop/hbase/oss/sync/ZKTreeLockManager.java
@@ -68,7 +68,7 @@ public class ZKTreeLockManager extends TreeLockManager {
     root = "/hboss";
   }
 
-  private static final String lockSubZnode = ".hboss-lock-znode";
+  public static final String LOCK_SUB_ZNODE = ".hboss-lock-znode";
 
   private Map<Path,InterProcessReadWriteLock> lockCache = new HashMap<>();
 
@@ -161,7 +161,7 @@ public class ZKTreeLockManager extends TreeLockManager {
       get(p).writeLock().release();
     } catch(IllegalMonitorStateException e) {
       // Reentrant locks might be acquired multiple times
-      LOG.error("Tried to release unacquired write lock: {}", p);
+      LOG.error("Tried to release unacquired write lock: {}", p, e);
       throw e;
     } catch (Exception e) {
       throw new IOException("Exception during write unlocking of path " + p, e);
@@ -226,10 +226,11 @@ public class ZKTreeLockManager extends TreeLockManager {
 
   @Override
   protected void recursiveDelete(Path p) throws IOException {
+    LOG.debug("Removing all mutex and znodes for paths beneath {}", p);
     try {
       ZKPaths.deleteChildren(curator.getZookeeperClient().getZooKeeper(),
             p.toString(), !p.isRoot());
-      // Before this method is called, we have a guarantee that 
+      // Before this method is called, we have a guarantee that
       //   1. There are no write locks above or below us
       //   2. There are no read locks below us
       // As such, we can just remove locks beneath us as we find them.
@@ -257,7 +258,7 @@ public class ZKTreeLockManager extends TreeLockManager {
    *
    * Specifically, this method will return true if the given path is a sub-directory
    * of the parent or a file in the directory represented by the parent. This method
-   * returns false if the parent and the given path are the same. 
+   * returns false if the parent and the given path are the same.
    */
   boolean isBeneath(Path parent, Path given) {
     if (parent.equals(given)) {
@@ -287,7 +288,7 @@ public class ZKTreeLockManager extends TreeLockManager {
       if (level < maxLevel) {
         List<String> children = curator.getChildren().forPath(p.toString());
         for (String child : children) {
-          if (child.equals(lockSubZnode)) {
+          if (child.equals(LOCK_SUB_ZNODE)) {
             continue;
           }
           if (writeLockBelow(new Path(p, child), level+1, maxLevel)) {
@@ -312,7 +313,7 @@ public class ZKTreeLockManager extends TreeLockManager {
       if (level < maxLevel) {
         List<String> children = curator.getChildren().forPath(p.toString());
         for (String child : children) {
-          if (child.equals(lockSubZnode)) {
+          if (child.equals(LOCK_SUB_ZNODE)) {
             continue;
           }
           if (readLockBelow(new Path(p, child), level+1, maxLevel)) {
@@ -357,6 +358,9 @@ public class ZKTreeLockManager extends TreeLockManager {
     StringBuilder sb = new StringBuilder();
     Map<Path,InterProcessReadWriteLock> cache = getUnmodifiableCache();
     for (Entry<Path,InterProcessReadWriteLock> entry : cache.entrySet()) {
+      if (sb.length() > 0) {
+        sb.append("\n");
+      }
       sb.append(entry.getKey()).append("=").append(describeLock(entry.getValue()));
     }
     return sb.toString();
@@ -382,7 +386,7 @@ public class ZKTreeLockManager extends TreeLockManager {
 
   private synchronized InterProcessReadWriteLock get(Path path) throws IOException {
     if (!lockCache.containsKey(path)) {
-      String zkPath = new Path(path, lockSubZnode).toString();
+      String zkPath = new Path(path, LOCK_SUB_ZNODE).toString();
       try {
         ZKPaths.mkdirs(curator.getZookeeperClient().getZooKeeper(), zkPath, true);
       } catch (KeeperException.NodeExistsException e) {
diff --git a/hbase-oss/src/test/java/org/apache/hadoop/hbase/oss/HBaseObjectStoreSemanticsTest.java b/hbase-oss/src/test/java/org/apache/hadoop/hbase/oss/HBaseObjectStoreSemanticsTest.java
index e11647b..a2c83f0 100644
--- a/hbase-oss/src/test/java/org/apache/hadoop/hbase/oss/HBaseObjectStoreSemanticsTest.java
+++ b/hbase-oss/src/test/java/org/apache/hadoop/hbase/oss/HBaseObjectStoreSemanticsTest.java
@@ -43,6 +43,10 @@ public class HBaseObjectStoreSemanticsTest {
     return TestUtils.testPath(hboss, path);
   }
 
+  public TreeLockManager getLockManager() {
+    return sync;
+  }
+
   @Before
   public void setup() throws Exception {
     Configuration conf = new Configuration();
diff --git a/hbase-oss/src/test/java/org/apache/hadoop/hbase/oss/TestZNodeCleanup.java b/hbase-oss/src/test/java/org/apache/hadoop/hbase/oss/TestZNodeCleanup.java
new file mode 100644
index 0000000..bf1dec6
--- /dev/null
+++ b/hbase-oss/src/test/java/org/apache/hadoop/hbase/oss/TestZNodeCleanup.java
@@ -0,0 +1,188 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.oss;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assume.assumeTrue;
+
+import java.util.List;
+import java.util.concurrent.CountDownLatch;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hbase.oss.sync.ZKTreeLockManager;
+import org.apache.zookeeper.WatchedEvent;
+import org.apache.zookeeper.Watcher;
+import org.apache.zookeeper.Watcher.Event.KeeperState;
+import org.apache.zookeeper.ZooKeeper;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Validates that when some entry in the object store is deleted, the corresponding
+ * data in ZooKeeper is also deleted.
+ */
+public class TestZNodeCleanup extends HBaseObjectStoreSemanticsTest {
+  private static final Logger LOG = LoggerFactory.getLogger(TestZNodeCleanup.class);
+
+  private ZKTreeLockManager lockManager;
+  private ZooKeeper zk;
+
+  @Before
+  public void setup() throws Exception {
+    super.setup();
+    assumeTrue("Lock manager is a " + getLockManager().getClass(),
+        getLockManager() instanceof ZKTreeLockManager);
+    lockManager = (ZKTreeLockManager) getLockManager();
+    Configuration conf = hboss.getConf();
+    LOG.info("Waiting for ZK client to connect");
+    // TODO should wait for ZK to connect
+    final CountDownLatch latch = new CountDownLatch(1);
+    // Root the ZK connection beneath /hboss
+    zk = new ZooKeeper(conf.get(Constants.ZK_CONN_STRING) + "/hboss", 60000, new Watcher() {
+      @Override
+      public void process(WatchedEvent event) {
+        LOG.info("Caught event " + event);
+        if (event.getState() == KeeperState.SyncConnected) {
+          latch.countDown();
+        }
+      }
+    });
+    latch.await();
+    LOG.info("ZooKeeper client is connected");
+  }
+
+  @After
+  public void teardown() throws Exception {
+    String zkRoot = lockManager.norm(TestUtils.testPathRoot(hboss)).toString();
+    LOG.info("Dumping contents of ZooKeeper after test from {}", zkRoot);
+    printZkBFS(zkRoot);
+    if (zk != null) {
+      zk.close();
+      zk = null;
+    }
+  }
+
+  void printZkBFS(String path) throws Exception {
+    LOG.info(path);
+    List<String> children = zk.getChildren(path, false);
+    for (String child : children) {
+      printZkBFS(path + "/" + child);
+    }
+  }
+
+  String getZNodeFromPath(Path p) {
+    return Path.getPathWithoutSchemeAndAuthority(p).toString().substring(1);
+  }
+
+  void validatePathInZk(Path zkPath) throws Exception {
+    assertNotNull(zkPath + " did not exist in ZK", zk.exists(zkPath.toString(), false));
+    String hbossLock = new Path(zkPath, ZKTreeLockManager.LOCK_SUB_ZNODE).toString();
+    assertNotNull(hbossLock + " did not exist in ZK", zk.exists(hbossLock, false));
+  }
+
+  void validatePathNotInZk(Path zkPath) throws Exception {
+    assertNull(zkPath + " incorrectly exists in ZK.", zk.exists(zkPath.toString(), false));
+  }
+
+  @Test
+  public void testRename() throws Exception {
+    // Rename src to dest and validate that the znode for src is cleaned up.
+    final Path src = TestUtils.testPath(hboss, "src");
+    final Path dest = TestUtils.testPath(hboss, "dest");
+    assertTrue(hboss.mkdirs(src));
+    // The src znode should exist after creating the dir in S3
+    validatePathInZk(lockManager.norm(src));
+    // `mv src dest`
+    assertTrue(hboss.rename(src, dest));
+    // We should have a znode for dest (we just locked it)
+    validatePathInZk(lockManager.norm(dest));
+    // We should no longer have a znode for src (we effectively deleted it from S3)
+    validatePathNotInZk(lockManager.norm(src));
+  }
+
+  @Test
+  public void testFailedRename() throws Exception {
+    // Rename src to dest and validate that the znode for src is cleaned up.
+    final Path src = TestUtils.testPathRoot(hboss);
+    final Path dest = TestUtils.testPath(hboss, "dest");
+    assertTrue(hboss.mkdirs(src));
+    // The src znode should exist after creating the dir in S3
+    validatePathInZk(lockManager.norm(src));
+    // The move should fail
+    assertFalse(hboss.rename(src, dest));
+    // We should have a znode for dest (we just locked it)
+    validatePathInZk(lockManager.norm(src));
+    // We should no longer have a znode for src (we effectively deleted it from S3)
+    validatePathInZk(lockManager.norm(dest));
+  }
+
+  @Test
+  public void testRenameDeeperToHigher() throws Exception {
+    // Rename src to dest and validate that the znode for src is cleaned up.
+    final Path src = TestUtils.testPath(hboss, "/a/b/1");
+    final Path dest = TestUtils.testPath(hboss, "/a/1");
+    assertTrue(hboss.mkdirs(src));
+    // The src znode should exist after creating the dir in S3
+    validatePathInZk(lockManager.norm(src));
+    // mv /a/b/1 /a/1
+    assertTrue(hboss.rename(src, dest));
+    // We should not have a znode for the src
+    validatePathNotInZk(lockManager.norm(src));
+    // We should have a lock for the dest
+    validatePathInZk(lockManager.norm(dest));
+  }
+
+  @Test
+  public void testRenameHigherToDeeper() throws Exception {
+    // Rename src to dest and validate that the znode for src is cleaned up.
+    final Path src = TestUtils.testPath(hboss, "/a/1");
+    final Path dest = TestUtils.testPath(hboss, "/a/b/1");
+    // `mkdir /a/1`
+    assertTrue(hboss.mkdirs(src));
+    // The src znode should exist after creating the dir in S3
+    validatePathInZk(lockManager.norm(src));
+    // `mkdir /a/b`
+    assertTrue(hboss.mkdirs(dest.getParent()));
+    // `mv /a/1 /a/b/1`
+    assertTrue(hboss.rename(src, dest));
+    // We should have a znode for dest (we just locked it)
+    validatePathNotInZk(lockManager.norm(src));
+    // We should no longer have a znode for src (we effectively deleted it from S3)
+    validatePathInZk(lockManager.norm(dest));
+  }
+
+  @Test
+  public void testDelete() throws Exception {
+    // Delete src and validate that the znode for src is cleaned up.
+    final Path src = TestUtils.testPath(hboss, "src");
+    assertTrue(hboss.mkdirs(src));
+    // The src znode should exist after creating the dir in S3
+    validatePathInZk(lockManager.norm(src));
+    // `mv src dest`
+    assertTrue(hboss.delete(src, true));
+    // We should no longer have a znode for src since we deleted it from S3
+    validatePathNotInZk(lockManager.norm(src));
+  }
+}
diff --git a/hbase-oss/src/test/java/org/apache/hadoop/hbase/oss/sync/TestTreeLockManager.java b/hbase-oss/src/test/java/org/apache/hadoop/hbase/oss/sync/TestTreeLockManager.java
index e9ef0ce..7236da8 100644
--- a/hbase-oss/src/test/java/org/apache/hadoop/hbase/oss/sync/TestTreeLockManager.java
+++ b/hbase-oss/src/test/java/org/apache/hadoop/hbase/oss/sync/TestTreeLockManager.java
@@ -36,7 +36,7 @@ public class TestTreeLockManager extends HBaseObjectStoreSemanticsTest {
 
   @Test
   public void testLockBelowChecks() throws Exception {
-    Assume.assumeFalse(sync instanceof NullTreeLockManager);
+    Assume.assumeFalse(getLockManager() instanceof NullTreeLockManager);
 
     Path parent = testPath("testListingLevels");
     Path child = new Path(parent, "child");
diff --git a/hbase-oss/src/test/resources/log4j.properties b/hbase-oss/src/test/resources/log4j.properties
index a59c82e..108b3f9 100644
--- a/hbase-oss/src/test/resources/log4j.properties
+++ b/hbase-oss/src/test/resources/log4j.properties
@@ -17,3 +17,5 @@ log4j.appender.stdout=org.apache.log4j.ConsoleAppender
 log4j.appender.stdout.layout=org.apache.log4j.PatternLayout
 log4j.appender.stdout.layout.ConversionPattern=%d{ISO8601} [%t] %-5p %c{2} (%F:%M(%L)) - %m%n
 log4j.logger.org.apache.hadoop=DEBUG
+log4j.logger.org.apache.hadoop.metrics2=WARN
+log4j.logger.org.apache.hadoop.fs=WARN
\ No newline at end of file