From c453fcb23fde0153cd8ebfa808c5728a2c8b99d4 Mon Sep 17 00:00:00 2001
From: Inigo Goiri <inigoiri@apache.org>
Date: Fri, 9 Mar 2018 17:18:51 -0800
Subject: [PATCH] HDFS-13212. RBF: Fix router location cache issue. Contributed
 by Weiwei Wu.

(cherry picked from commit afe1a3ccd56a12fec900360a8a2855c080728e65)
---
 .../resolver/MountTableResolver.java          | 15 ++++--
 .../resolver/TestMountTableResolver.java      | 46 +++++++++++++++++++
 2 files changed, 58 insertions(+), 3 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 dac6f7f424f..2c7d1f88ebd 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
@@ -238,9 +238,17 @@ private void invalidateLocationCache(final String path) {
       Entry<String, PathLocation> entry = it.next();
       PathLocation loc = entry.getValue();
       String src = loc.getSourcePath();
-      if (src.startsWith(path)) {
-        LOG.debug("Removing {}", src);
-        it.remove();
+      if (src != null) {
+        if (src.startsWith(path)) {
+          LOG.debug("Removing {}", src);
+          it.remove();
+        }
+      } else {
+        String dest = loc.getDefaultLocation().getDest();
+        if (dest.startsWith(path)) {
+          LOG.debug("Removing default cache {}", dest);
+          it.remove();
+        }
       }
     }
 
@@ -287,6 +295,7 @@ public void refreshEntries(final Collection<MountTable> entries) {
         if (!oldEntries.contains(srcPath)) {
           // Add node, it does not exist
           this.tree.put(srcPath, entry);
+          invalidateLocationCache(srcPath);
           LOG.info("Added new mount point {} to resolver", srcPath);
         } else {
           // Node exists, check for updates
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/federation/resolver/TestMountTableResolver.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/federation/resolver/TestMountTableResolver.java
index a09daf0975b..f530fe99c42 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/federation/resolver/TestMountTableResolver.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/federation/resolver/TestMountTableResolver.java
@@ -17,6 +17,7 @@
  */
 package org.apache.hadoop.hdfs.server.federation.resolver;
 
+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.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
@@ -82,6 +83,7 @@ private void setupMountTable() throws IOException {
     Configuration conf = new Configuration();
     conf.setInt(
         FEDERATION_MOUNT_TABLE_MAX_CACHE_SIZE, TEST_MAX_CACHE_SIZE);
+    conf.setStrings(DFS_ROUTER_DEFAULT_NAMESERVICE, "0");
     mountTable = new MountTableResolver(conf);
 
     // Root mount point
@@ -479,4 +481,48 @@ public void testCacheCleaning() throws Exception {
     long cacheSize = mountTable.getCacheSize();
     assertTrue(cacheSize <= TEST_MAX_CACHE_SIZE);
   }
+
+  @Test
+  public void testLocationCache() throws Exception {
+    List<MountTable> entries = new ArrayList<>();
+
+    // Add entry and test location cache
+    Map<String, String> map1 = getMountTableEntry("1", "/testlocationcache");
+    MountTable entry1 = MountTable.newInstance("/testlocationcache", map1);
+    entries.add(entry1);
+
+    Map<String, String> map2 = getMountTableEntry("2",
+            "/anothertestlocationcache");
+    MountTable entry2 = MountTable.newInstance("/anothertestlocationcache",
+            map2);
+    entries.add(entry2);
+    mountTable.refreshEntries(entries);
+    assertEquals("1->/testlocationcache/",
+            mountTable.getDestinationForPath("/testlocationcache").toString());
+    assertEquals("2->/anothertestlocationcache/",
+            mountTable.getDestinationForPath("/anothertestlocationcache")
+                    .toString());
+
+    // Remove the entry1
+    entries.remove(entry1);
+    mountTable.refreshEntries(entries);
+
+    // Add the default location and test location cache
+    assertEquals("0->/testlocationcache",
+            mountTable.getDestinationForPath("/testlocationcache").toString());
+
+    // Add the entry again but mount to another ns
+    Map<String, String> map3 = getMountTableEntry("3", "/testlocationcache");
+    MountTable entry3 = MountTable.newInstance("/testlocationcache", map3);
+    entries.add(entry3);
+    mountTable.refreshEntries(entries);
+
+    // Ensure location cache update correctly
+    assertEquals("3->/testlocationcache/",
+            mountTable.getDestinationForPath("/testlocationcache").toString());
+
+    // Cleanup before exit
+    mountTable.removeEntry("/testlocationcache");
+    mountTable.removeEntry("/anothertestlocationcache");
+  }
 }
\ No newline at end of file