HBASE-26530 Backport HBASE-26524 Support remove coprocessor by class …

…name via alter table command (#3908)

 Signed-off-by: Ankit Singhal <[email protected]>

hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableDescriptorBuilder.java

@@ -1600,6 +1600,8 @@ public void removeCoprocessor(String className) {
       // if we found a match, remove it
       if (match != null) {
         ModifyableTableDescriptor.this.removeValue(match);
+      } else {
+        LOG.warn("coprocessor with class name {} was not found in the table attribute", className);
       }
     }
 

hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestTableDescriptorBuilder.java

@@ -158,6 +158,20 @@ public void testSetListRemoveCP() throws Exception {
       .anyMatch(name -> name.equals(className2)));
   }
 
+  /**
+   * Test removing cps in the table description that does not exist
+   */
+  @Test
+  public void testRemoveNonExistingCoprocessor() throws Exception {
+    String className = "org.apache.hadoop.hbase.coprocessor.SimpleRegionObserver";
+    TableDescriptor desc = TableDescriptorBuilder
+      .newBuilder(TableName.valueOf(name.getMethodName()))
+      .build();
+    assertFalse(desc.hasCoprocessor(className));
+    desc = TableDescriptorBuilder.newBuilder(desc).removeCoprocessor(className).build();
+    assertFalse(desc.hasCoprocessor(className));
+  }
+
   /**
    * Test that we add and remove strings from settings properly.
    */

hbase-shell/src/main/ruby/hbase/admin.rb

@@ -758,6 +758,17 @@ def alter(table_name_str, wait = true, *args)
               htd.remove(name)
             end
             hasTableUpdate = true
+          elsif method == 'table_remove_coprocessor'
+            classname = arg.delete(CLASSNAME)
+            raise(ArgumentError, 'CLASSNAME parameter missing for table_remove_coprocessor method') unless classname
+            if classname.is_a?(Array)
+              classname.each do |key|
+                htd.removeCoprocessor(key)
+              end
+            else
+              htd.removeCoprocessor(classname)
+            end
+            hasTableUpdate = true
           # Unset table configuration
           elsif method == 'table_conf_unset'
             raise(ArgumentError, 'NAME parameter missing for table_conf_unset method') unless name

hbase-shell/src/main/ruby/hbase_constants.rb

@@ -39,6 +39,7 @@ module HBaseConstants
   BATCH = 'BATCH'.freeze
   CACHE = 'CACHE'.freeze
   CACHE_BLOCKS = 'CACHE_BLOCKS'.freeze
+  CLASSNAME = 'CLASSNAME'.freeze
   CLUSTER_KEY = 'CLUSTER_KEY'.freeze
   COLUMN = 'COLUMN'.freeze
   COLUMNS = 'COLUMNS'.freeze

hbase-shell/src/main/ruby/shell/commands/alter.rb

@@ -82,6 +82,18 @@ def help
 
   hbase> alter 't1', METHOD => 'table_att_unset', NAME => 'coprocessor$1'
 
+Other than removing coprocessor from the table-scope attribute via 'table_att_unset', you can also
+use 'table_remove_coprocessor' by specifying the class name:
+
+  hbase> alter 't1', METHOD => 'table_remove_coprocessor', CLASSNAME =>
+           'org.apache.hadoop.hbase.coprocessor.SimpleRegionObserver'
+
+You can also remove multiple coprocessors at once:
+
+  hbase> alter 't1', METHOD => 'table_remove_coprocessor', CLASSNAME =>
+           ['org.apache.hadoop.hbase.coprocessor.SimpleRegionObserver',
+            'org.apache.hadoop.hbase.coprocessor.Export']
+
 You can also set REGION_REPLICATION:
 
   hbase> alter 't1', {REGION_REPLICATION => 2}

hbase-shell/src/test/ruby/hbase/admin_test.rb

@@ -977,6 +977,25 @@ def teardown
       assert_no_match(eval("/" + key + "/"), admin.describe(@test_name))
     end
 
+    define_test "alter should be able to remove a coprocessor by class name" do
+      drop_test_table(@test_name)
+      create_test_table(@test_name)
+
+      cp_key = "coprocessor"
+      class_name = "org.apache.hadoop.hbase.coprocessor.SimpleRegionObserver"
+      cp_value = "|" + class_name + "|12|arg1=1,arg2=2"
+
+      command(:alter, @test_name, 'METHOD' => 'table_att', cp_key => cp_value)
+      describe_text = admin.describe(@test_name)
+      assert_match(eval("/" + class_name + "/"), describe_text)
+      assert_match(eval("/" + cp_key + "\\$(\\d+)/"), describe_text)
+      assert_match(/arg1=1,arg2=2/, describe_text)
+
+      command(:alter, @test_name, 'METHOD' => 'table_remove_coprocessor', 'CLASSNAME' => class_name)
+      describe_text = admin.describe(@test_name)
+      assert_no_match(eval("/" + class_name + "/"), describe_text)
+    end
+
     define_test "alter should be able to remove a list of table attributes" do
       drop_test_table(@test_name)