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 <els...@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 <wchevre...@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"))); + } +}