From 7fab787de72756863a91c2358da5c611afdb80e9 Mon Sep 17 00:00:00 2001 From: Yiqun Lin Date: Tue, 13 Mar 2018 10:30:20 +0800 Subject: [PATCH] HDFS-13253. RBF: Quota management incorrect parent-child relationship judgement. Contributed by Yiqun Lin. --- .../resolver/MountTableResolver.java | 14 ++----------- .../federation/router/FederationUtil.java | 20 +++++++++++++++++++ .../federation/router/RouterQuotaManager.java | 14 ++++++++++++- .../router/TestRouterQuotaManager.java | 12 +++++++++++ 4 files changed, 47 insertions(+), 13 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/federation/resolver/MountTableResolver.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/federation/resolver/MountTableResolver.java index 2c7d1f88eb..27b43e54f2 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/federation/resolver/MountTableResolver.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/federation/resolver/MountTableResolver.java @@ -20,6 +20,7 @@ import static org.apache.hadoop.hdfs.DFSConfigKeys.DFS_ROUTER_DEFAULT_NAMESERVICE; import static org.apache.hadoop.hdfs.DFSConfigKeys.FEDERATION_MOUNT_TABLE_MAX_CACHE_SIZE; import static org.apache.hadoop.hdfs.DFSConfigKeys.FEDERATION_MOUNT_TABLE_MAX_CACHE_SIZE_DEFAULT; +import static org.apache.hadoop.hdfs.server.federation.router.FederationUtil.isParentEntry; import java.io.IOException; import java.util.Collection; @@ -239,7 +240,7 @@ private void invalidateLocationCache(final String path) { PathLocation loc = entry.getValue(); String src = loc.getSourcePath(); if (src != null) { - if (src.startsWith(path)) { + if(isParentEntry(src, path)) { LOG.debug("Removing {}", src); it.remove(); } @@ -530,17 +531,6 @@ public String getDefaultNamespace() { return this.defaultNameService; } - private boolean isParentEntry(final String path, final String parent) { - if (!path.startsWith(parent)) { - return false; - } - if (path.equals(parent)) { - return true; - } - return path.charAt(parent.length()) == Path.SEPARATOR_CHAR - || parent.equals(Path.SEPARATOR); - } - /** * Find the deepest mount point for a path. * @param path Path to look for. diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/federation/router/FederationUtil.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/federation/router/FederationUtil.java index 8d631e99ee..3dfd99809e 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/federation/router/FederationUtil.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/federation/router/FederationUtil.java @@ -26,6 +26,7 @@ import java.net.URLConnection; import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.Path; import org.apache.hadoop.hdfs.DFSConfigKeys; import org.apache.hadoop.hdfs.server.federation.resolver.ActiveNamenodeResolver; import org.apache.hadoop.hdfs.server.federation.resolver.FileSubclusterResolver; @@ -186,4 +187,23 @@ public static ActiveNamenodeResolver newActiveNamenodeResolver( ActiveNamenodeResolver.class); return newInstance(conf, stateStore, StateStoreService.class, clazz); } + + /** + * Check if the given path is the child of parent path. + * @param path Path to be check. + * @param parent Parent path. + * @return True if parent path is parent entry for given path. + */ + public static boolean isParentEntry(final String path, final String parent) { + if (!path.startsWith(parent)) { + return false; + } + + if (path.equals(parent)) { + return true; + } + + return path.charAt(parent.length()) == Path.SEPARATOR_CHAR + || parent.equals(Path.SEPARATOR); + } } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterQuotaManager.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterQuotaManager.java index fc3575c63d..0df34fc641 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterQuotaManager.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterQuotaManager.java @@ -17,6 +17,9 @@ */ package org.apache.hadoop.hdfs.server.federation.router; +import static org.apache.hadoop.hdfs.server.federation.router.FederationUtil.isParentEntry; + +import java.util.HashSet; import java.util.Set; import java.util.SortedMap; import java.util.TreeMap; @@ -94,7 +97,16 @@ public Set getPaths(String parentPath) { String from = parentPath; String to = parentPath + Character.MAX_VALUE; SortedMap subMap = this.cache.subMap(from, to); - return subMap.keySet(); + + Set validPaths = new HashSet<>(); + if (subMap != null) { + for (String path : subMap.keySet()) { + if (isParentEntry(path, parentPath)) { + validPaths.add(path); + } + } + } + return validPaths; } finally { readLock.unlock(); } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterQuotaManager.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterQuotaManager.java index 346c881477..ce3ee17475 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterQuotaManager.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterQuotaManager.java @@ -57,6 +57,18 @@ public void testGetChildrenPaths() { assertTrue(childrenPaths.contains("/path1/subdir") && childrenPaths.contains("/path1/subdir/subdir") && childrenPaths.contains("/path1")); + + // test for corner case + manager.put("/path3", quotaUsage); + manager.put("/path3/subdir", quotaUsage); + manager.put("/path3-subdir", quotaUsage); + + childrenPaths = manager.getPaths("/path3"); + assertEquals(2, childrenPaths.size()); + // path /path3-subdir should not be returned + assertTrue(childrenPaths.contains("/path3") + && childrenPaths.contains("/path3/subdir") + && !childrenPaths.contains("/path3-subdir")); } @Test