From 318298047be788143405cfb101e1b0401f9e6b7a Mon Sep 17 00:00:00 2001
From: tedyu <yuzhihong@gmail.com>
Date: Tue, 14 Mar 2017 12:34:04 -0700
Subject: [PATCH] HBASE-17779 disable_table_replication returns misleading
 message and does not turn off replication (Janos Gub)

---
 .../client/replication/ReplicationAdmin.java  | 37 ++++++++++++++-----
 .../TestReplicationAdminWithClusters.java     | 17 +++++++++
 2 files changed, 45 insertions(+), 9 deletions(-)

diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java
index f9cf6b331e5..80f6e1065d6 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/replication/ReplicationAdmin.java
@@ -647,15 +647,17 @@ public class ReplicationAdmin implements Closeable {
   /**
    * Set the table's replication switch if the table's replication switch is already not set.
    * @param tableName name of the table
-   * @param isRepEnabled is replication switch enable or disable
+   * @param enableRep is replication switch enable or disable
    * @throws IOException if a remote or network exception occurs
    */
-  private void setTableRep(final TableName tableName, boolean isRepEnabled) throws IOException {
+  private void setTableRep(final TableName tableName, boolean enableRep) throws IOException {
     Admin admin = null;
     try {
       admin = this.connection.getAdmin();
       HTableDescriptor htd = admin.getTableDescriptor(tableName);
-      if (isTableRepEnabled(htd) ^ isRepEnabled) {
+      ReplicationState currentReplicationState = getTableReplicationState(htd);
+      if (enableRep && currentReplicationState != ReplicationState.ENABLED
+          || !enableRep && currentReplicationState != ReplicationState.DISABLED) {
         boolean isOnlineSchemaUpdateEnabled =
             this.connection.getConfiguration()
                 .getBoolean("hbase.online.schema.update.enable", true);
@@ -663,7 +665,7 @@ public class ReplicationAdmin implements Closeable {
           admin.disableTable(tableName);
         }
         for (HColumnDescriptor hcd : htd.getFamilies()) {
-          hcd.setScope(isRepEnabled ? HConstants.REPLICATION_SCOPE_GLOBAL
+          hcd.setScope(enableRep ? HConstants.REPLICATION_SCOPE_GLOBAL
               : HConstants.REPLICATION_SCOPE_LOCAL);
         }
         admin.modifyTable(tableName, htd);
@@ -684,17 +686,34 @@ public class ReplicationAdmin implements Closeable {
   }
 
   /**
-   * @param htd table descriptor details for the table to check
-   * @return true if table's replication switch is enabled
+   * This enum indicates the current state of the replication for a given table.
    */
-  private boolean isTableRepEnabled(HTableDescriptor htd) {
+  private enum ReplicationState {
+    ENABLED, // all column families enabled
+    MIXED, // some column families enabled, some disabled
+    DISABLED // all column families disabled
+  }
+
+  /**
+   * @param htd table descriptor details for the table to check
+   * @return ReplicationState the current state of the table.
+   */
+  private ReplicationState getTableReplicationState(HTableDescriptor htd) {
+    boolean hasEnabled = false;
+    boolean hasDisabled = false;
+
     for (HColumnDescriptor hcd : htd.getFamilies()) {
       if (hcd.getScope() != HConstants.REPLICATION_SCOPE_GLOBAL
           && hcd.getScope() != HConstants.REPLICATION_SCOPE_SERIAL) {
-        return false;
+        hasDisabled = true;
+      } else {
+        hasEnabled = true;
       }
     }
-    return true;
+
+    if (hasEnabled && hasDisabled) return ReplicationState.MIXED;
+    if (hasEnabled) return ReplicationState.ENABLED;
+    return ReplicationState.DISABLED;
   }
 
   private void checkConfiguredWALEntryFilters(ReplicationPeerConfig peerConfig)
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/replication/TestReplicationAdminWithClusters.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/replication/TestReplicationAdminWithClusters.java
index 26ed7a7a6fa..1c2c36d351b 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/replication/TestReplicationAdminWithClusters.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/replication/TestReplicationAdminWithClusters.java
@@ -69,6 +69,23 @@ public class TestReplicationAdminWithClusters extends TestReplicationBase {
     TestReplicationBase.tearDownAfterClass();
   }
 
+  @Test(timeout = 300000)
+  public void disableNotFullReplication() throws Exception {
+    HTableDescriptor table = admin2.getTableDescriptor(tableName);
+    HColumnDescriptor f = new HColumnDescriptor("notReplicatedFamily");
+    table.addFamily(f);
+    admin1.disableTable(tableName);
+    admin1.modifyTable(tableName, table);
+    admin1.enableTable(tableName);
+
+
+    admin.disableTableRep(tableName);
+    table = admin1.getTableDescriptor(tableName);
+    for (HColumnDescriptor fam : table.getColumnFamilies()) {
+      assertEquals(fam.getScope(), HConstants.REPLICATION_SCOPE_LOCAL);
+    }
+  }
+
   @Test(timeout = 300000)
   public void testEnableReplicationWhenSlaveClusterDoesntHaveTable() throws Exception {
     admin.disableTableRep(tableName);