HBASE-26524 Support remove coprocessor by class name via alter table command (apache#3902)

Signed-off-by: Nick Dimiduk <[email protected]>
Signed-off-by: Wellington Chevreuil <[email protected]>

Author: taklwu

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

@@ -1561,6 +1561,10 @@ public void removeCoprocessor(String className) {
       // if we found a match, remove it
       if (match != null) {
         ModifyableTableDescriptor.this.removeValue(match);
+      } else {
+        throw new IllegalArgumentException(String
+          .format("coprocessor with class name %s was not found in the table attribute",
+            className));
       }
     }
 

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

@@ -148,6 +148,20 @@ public void testSetListRemoveCP() throws Exception {
       .anyMatch(name -> name.equals(className2)));
   }
 
+  /**
+   * Test removing cps in the table description that does not exist
+   * @throws Exception if removing a coprocessor fails other than IllegalArgumentException
+   */
+  @Test(expected = IllegalArgumentException.class)
+  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));
+    TableDescriptorBuilder.newBuilder(desc).removeCoprocessor(className).build();
+  }
+
   /**
    * Test that we add and remove strings from settings properly.
    */

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

@@ -824,6 +824,17 @@ def alter(table_name_str, wait = true, *args)
               tdb.removeValue(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|
+                tdb.removeCoprocessor(key)
+              end
+            else
+              tdb.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/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

@@ -1010,6 +1010,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)