From 8eba922bb3860e9a819953964fc6ba3bd221b47d Mon Sep 17 00:00:00 2001
From: Ayush Saxena <ayushsaxena@apache.org>
Date: Sun, 20 Sep 2020 08:54:25 +0530
Subject: [PATCH] HDFS-15579. RBF: The constructor of PathLocation may got some
 misunderstanding. Contributed by Janus Chow.

---
 .../MultipleDestinationMountTableResolver.java   |  5 +++--
 .../server/federation/resolver/PathLocation.java | 16 ++++++++++++++++
 .../TestMultipleDestinationResolver.java         |  9 +++++++++
 3 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/resolver/MultipleDestinationMountTableResolver.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/resolver/MultipleDestinationMountTableResolver.java
index b09a883ebc..752f5c951f 100644
--- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/resolver/MultipleDestinationMountTableResolver.java
+++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/resolver/MultipleDestinationMountTableResolver.java
@@ -99,8 +99,9 @@ public PathLocation getDestinationForPath(String path) throws IOException {
 
         // Change the order of the name spaces according to the policy
         if (firstNamespace != null) {
-          // This is the entity in the tree, we need to create our own copy
-          mountTableResult = new PathLocation(mountTableResult, firstNamespace);
+          // Create our own prioritized copy based on the entity in the tree.
+          mountTableResult = PathLocation.prioritizeDestination(
+              mountTableResult, firstNamespace);
           LOG.debug("Ordered locations following {} are {}",
               order, mountTableResult);
         } else {
diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/resolver/PathLocation.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/resolver/PathLocation.java
index cb04ff8f93..44856cb5e8 100644
--- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/resolver/PathLocation.java
+++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/resolver/PathLocation.java
@@ -87,12 +87,28 @@ public PathLocation(final PathLocation other) {
    * @param other Other path location to copy from.
    * @param firstNsId Identifier of the namespace to place first.
    */
+  @Deprecated
   public PathLocation(PathLocation other, String firstNsId) {
     this.sourcePath = other.sourcePath;
     this.destOrder = other.destOrder;
     this.destinations = orderedNamespaces(other.destinations, firstNsId);
   }
 
+  /**
+   * Return a path location with the prioritized destinations based on
+   * the current path location.
+   *
+   * @param base The base path location we'd like to prioritize on.
+   * @param firstNsId Identifier of the namespace to place first.
+   */
+  public static PathLocation prioritizeDestination(
+      PathLocation base, String firstNsId) {
+    List<RemoteLocation> prioritizedDestinations = orderedNamespaces(
+        base.destinations, firstNsId);
+    return new PathLocation(base.sourcePath, prioritizedDestinations,
+        base.destOrder);
+  }
+
   /**
    * Prioritize a location/destination by its name space/nameserviceId.
    * This destination might be used by other threads, so the source is not
diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/resolver/TestMultipleDestinationResolver.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/resolver/TestMultipleDestinationResolver.java
index 3915c564bf..c4b211c6ee 100644
--- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/resolver/TestMultipleDestinationResolver.java
+++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/resolver/TestMultipleDestinationResolver.java
@@ -360,6 +360,15 @@ public void testRandomResolver() throws IOException {
     assertEquals(3, destinations.size());
   }
 
+  @Test
+  public void testPrioritizeDestination() throws IOException {
+    PathLocation dest0 = resolver.getDestinationForPath("/hashall/file0.txt");
+    assertDest("subcluster0", dest0);
+    PathLocation prioritizedDest = PathLocation.prioritizeDestination(dest0,
+        "subcluster1");
+    assertDest("subcluster1", prioritizedDest);
+  }
+
   /**
    * Test that a path has files distributed across destinations evenly.
    * @param path Path to check.