HDFS-14777. RBF: Set ReadOnly is failing for mount Table but actually readonly succeed to set. Contributed by Ranith Sardar.

hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterAdminServer.java

@@ -30,6 +30,7 @@ import java.util.Map;
 import java.util.Set;
 
 import com.google.common.base.Preconditions;
+
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hdfs.DFSConfigKeys;
 import org.apache.hadoop.hdfs.DFSUtil;
@@ -42,6 +43,7 @@ import org.apache.hadoop.hdfs.protocolPB.RouterAdminProtocolServerSideTranslator
 import org.apache.hadoop.hdfs.protocolPB.RouterPolicyProvider;
 import org.apache.hadoop.hdfs.server.federation.resolver.ActiveNamenodeResolver;
 import org.apache.hadoop.hdfs.server.federation.resolver.FederationNamespaceInfo;
+import org.apache.hadoop.hdfs.server.federation.resolver.MountTableResolver;
 import org.apache.hadoop.hdfs.server.federation.resolver.RemoteLocation;
 import org.apache.hadoop.hdfs.server.federation.store.DisabledNameserviceStore;
 import org.apache.hadoop.hdfs.server.federation.store.MountTableStore;
@@ -267,11 +269,13 @@ public class RouterAdminServer extends AbstractService
   @Override
   public UpdateMountTableEntryResponse updateMountTableEntry(
       UpdateMountTableEntryRequest request) throws IOException {
-    UpdateMountTableEntryResponse response =
+
-        getMountTableStore().updateMountTableEntry(request);
+    UpdateMountTableEntryResponse response = getMountTableStore()
+        .updateMountTableEntry(request);
     try {
       MountTable mountTable = request.getEntry();
-      if (mountTable != null && router.isQuotaEnabled()) {
+      if (mountTable != null && router.isQuotaEnabled()
+          && isQuotaUpdated(request, mountTable)) {
         synchronizeQuota(mountTable.getSourcePath(),
             mountTable.getQuota().getQuota(),
             mountTable.getQuota().getSpaceQuota());
@@ -280,11 +284,36 @@ public class RouterAdminServer extends AbstractService
       // Ignore exception, if any while reseting quota. Specifically to handle
       // if the actual destination doesn't exist.
       LOG.warn("Unable to reset quota at the destinations for {}: {}",
-          request.getEntry().toString(), e.getMessage());
+          request.getEntry(), e.getMessage());
     }
     return response;
   }
 
+  private boolean isQuotaUpdated(UpdateMountTableEntryRequest request,
+      MountTable mountTable) throws IOException {
+    long nsQuota = -1;
+    long ssQuota = -1;
+
+    String path = request.getEntry().getSourcePath();
+    if (this.router.getSubclusterResolver() instanceof MountTableResolver) {
+      MountTableResolver mResolver = (MountTableResolver) this.router
+          .getSubclusterResolver();
+      MountTable entry = mResolver.getMountPoint(path);
+      if (entry != null) {
+        RouterQuotaUsage preQuota = entry.getQuota();
+        nsQuota = preQuota.getQuota();
+        ssQuota = preQuota.getSpaceQuota();
+      }
+    }
+    RouterQuotaUsage mountQuota = mountTable.getQuota();
+    if (nsQuota != mountQuota.getQuota()
+        || ssQuota != mountQuota.getSpaceQuota()) {
+      return true;
+    }
+
+    return false;
+  }
+
   /**
    * Synchronize the quota value across mount table and subclusters.
    * @param path Source path in given mount table.

hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterAdminCLI.java

@@ -93,6 +93,7 @@ public class TestRouterAdminCLI {
         .metrics()
         .admin()
         .rpc()
+        .quota()
         .safemode()
         .build();
     cluster.addRouterOverrides(conf);
@@ -1209,6 +1210,55 @@ public class TestRouterAdminCLI {
     assertEquals((short)0455, mountTable.getMode().toShort());
   }
 
+  @Test
+  public void testUpdateReadonlyWithQuota() throws Exception {
+    // Add a mount table
+    String nsId = "ns0";
+    String src = "/test-updateReadonlywithQuota";
+    String dest = "/UpdateReadonlywithQuota";
+    String[] argv = new String[] {"-add", src, nsId, dest };
+    assertEquals(0, ToolRunner.run(admin, argv));
+
+    stateStore.loadCache(MountTableStoreImpl.class, true);
+    GetMountTableEntriesRequest getRequest = GetMountTableEntriesRequest
+        .newInstance(src);
+    GetMountTableEntriesResponse getResponse = client.getMountTableManager()
+        .getMountTableEntries(getRequest);
+    // Ensure mount table added successfully
+    MountTable mountTable = getResponse.getEntries().get(0);
+    assertEquals(src, mountTable.getSourcePath());
+    RemoteLocation localDest = mountTable.getDestinations().get(0);
+    assertEquals(nsId, localDest.getNameserviceId());
+    assertEquals(dest, localDest.getDest());
+    assertFalse(mountTable.isReadOnly());
+
+    argv = new String[] {"-update", src, nsId, dest, "-readonly", "true" };
+    assertEquals(0, ToolRunner.run(admin, argv));
+
+    stateStore.loadCache(MountTableStoreImpl.class, true);
+    getResponse = client.getMountTableManager()
+        .getMountTableEntries(getRequest);
+    mountTable = getResponse.getEntries().get(0);
+    assertTrue(mountTable.isReadOnly());
+
+    // Update Quota
+    long nsQuota = 50;
+    long ssQuota = 100;
+    argv = new String[] {"-setQuota", src, "-nsQuota", String.valueOf(nsQuota),
+        "-ssQuota", String.valueOf(ssQuota) };
+    assertEquals(0, ToolRunner.run(admin, argv));
+
+    stateStore.loadCache(MountTableStoreImpl.class, true);
+    getResponse = client.getMountTableManager()
+        .getMountTableEntries(getRequest);
+
+    mountTable = getResponse.getEntries().get(0);
+    RouterQuotaUsage quota = mountTable.getQuota();
+    assertEquals(nsQuota, quota.getQuota());
+    assertEquals(ssQuota, quota.getSpaceQuota());
+    assertTrue(mountTable.isReadOnly());
+  }
+
   @Test
   public void testUpdateOrderMountTable() throws Exception {
     testUpdateOrderMountTable(DestinationOrder.HASH);