From 72c320dec275b7ae4633e75b5afb4951c1adc9db Mon Sep 17 00:00:00 2001 From: "Tak Lon (Stephen) Wu" Date: Wed, 1 Dec 2021 14:47:59 -0800 Subject: [PATCH 1/4] HBASE-26530 Backport HBASE-26524 Support remove coprocessor by class name via alter table command we need one line from HBASE-24874 for setting up constant CLASSNAME original commit 19b0b2e8fc4877e11dfa8fd34bacf5db0bc5b1ad --- .../hbase/client/TableDescriptorBuilder.java | 4 ++++ .../client/TestTableDescriptorBuilder.java | 14 ++++++++++++++ hbase-shell/src/main/ruby/hbase/admin.rb | 11 +++++++++++ hbase-shell/src/main/ruby/hbase_constants.rb | 1 + .../src/main/ruby/shell/commands/alter.rb | 12 ++++++++++++ hbase-shell/src/test/ruby/hbase/admin_test.rb | 19 +++++++++++++++++++ 6 files changed, 61 insertions(+) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableDescriptorBuilder.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableDescriptorBuilder.java index 01d275310e93..110b10b4b379 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableDescriptorBuilder.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableDescriptorBuilder.java @@ -1600,6 +1600,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)); } } diff --git a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestTableDescriptorBuilder.java b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestTableDescriptorBuilder.java index aa4d5a1b5b53..cb4777c49aae 100644 --- a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestTableDescriptorBuilder.java +++ b/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 + * @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. */ diff --git a/hbase-shell/src/main/ruby/hbase/admin.rb b/hbase-shell/src/main/ruby/hbase/admin.rb index c1da4bbb52b8..f4cfd31716ed 100644 --- a/hbase-shell/src/main/ruby/hbase/admin.rb +++ b/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 diff --git a/hbase-shell/src/main/ruby/hbase_constants.rb b/hbase-shell/src/main/ruby/hbase_constants.rb index c7b133eaba60..3c637b88029d 100644 --- a/hbase-shell/src/main/ruby/hbase_constants.rb +++ b/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 diff --git a/hbase-shell/src/main/ruby/shell/commands/alter.rb b/hbase-shell/src/main/ruby/shell/commands/alter.rb index 93323bef7d36..9d95bffbdda6 100644 --- a/hbase-shell/src/main/ruby/shell/commands/alter.rb +++ b/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} diff --git a/hbase-shell/src/test/ruby/hbase/admin_test.rb b/hbase-shell/src/test/ruby/hbase/admin_test.rb index 26793a121926..e84ffd6385a7 100644 --- a/hbase-shell/src/test/ruby/hbase/admin_test.rb +++ b/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) From 52c68aee21e26a4c516b7d8465749cd73fdb5001 Mon Sep 17 00:00:00 2001 From: "Tak Lon (Stephen) Wu" Date: Thu, 2 Dec 2021 12:55:41 -0800 Subject: [PATCH 2/4] Backport HBASE-26524 Addendum fix TestConstraints --- .../org/apache/hadoop/hbase/constraint/Constraints.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/constraint/Constraints.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/constraint/Constraints.java index 759d2520b0be..9ad077432701 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/constraint/Constraints.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/constraint/Constraints.java @@ -103,7 +103,11 @@ public static void enable(HTableDescriptor desc) throws IOException { * Constraints}. */ public static void disable(HTableDescriptor desc) { - desc.removeCoprocessor(ConstraintProcessor.class.getName()); + try { + desc.removeCoprocessor(ConstraintProcessor.class.getName()); + } catch (IllegalArgumentException e) { + LOG.warn("ConstraintProcessor was unset.", e); + } } /** From 6e2bdac7156bdc9f0567540efe1a9cf584c12e03 Mon Sep 17 00:00:00 2001 From: "Tak Lon (Stephen) Wu" Date: Wed, 8 Dec 2021 13:13:29 -0800 Subject: [PATCH 3/4] keep branch-2 compatible without IllegalArgumentException --- .../apache/hadoop/hbase/client/TableDescriptorBuilder.java | 4 +--- .../hadoop/hbase/client/TestTableDescriptorBuilder.java | 5 ++--- .../org/apache/hadoop/hbase/constraint/Constraints.java | 6 +----- 3 files changed, 4 insertions(+), 11 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableDescriptorBuilder.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableDescriptorBuilder.java index 110b10b4b379..c2ca5a1d59b4 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableDescriptorBuilder.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableDescriptorBuilder.java @@ -1601,9 +1601,7 @@ public void removeCoprocessor(String className) { 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)); + LOG.warn("coprocessor with class name {} was not found in the table attribute", className); } } diff --git a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestTableDescriptorBuilder.java b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestTableDescriptorBuilder.java index cb4777c49aae..8d5d68d93335 100644 --- a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestTableDescriptorBuilder.java +++ b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestTableDescriptorBuilder.java @@ -160,16 +160,15 @@ public void testSetListRemoveCP() throws Exception { /** * 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(); + desc = TableDescriptorBuilder.newBuilder(desc).removeCoprocessor(className).build(); + assertFalse(desc.hasCoprocessor(className)); } /** diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/constraint/Constraints.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/constraint/Constraints.java index 9ad077432701..759d2520b0be 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/constraint/Constraints.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/constraint/Constraints.java @@ -103,11 +103,7 @@ public static void enable(HTableDescriptor desc) throws IOException { * Constraints}. */ public static void disable(HTableDescriptor desc) { - try { - desc.removeCoprocessor(ConstraintProcessor.class.getName()); - } catch (IllegalArgumentException e) { - LOG.warn("ConstraintProcessor was unset.", e); - } + desc.removeCoprocessor(ConstraintProcessor.class.getName()); } /** From e943488d00d67421b2d4127b8d2a4579cd1541eb Mon Sep 17 00:00:00 2001 From: "Tak Lon (Stephen) Wu" Date: Wed, 8 Dec 2021 17:18:16 -0800 Subject: [PATCH 4/4] fix a missing @Test annotations --- .../apache/hadoop/hbase/client/TestTableDescriptorBuilder.java | 1 + 1 file changed, 1 insertion(+) diff --git a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestTableDescriptorBuilder.java b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestTableDescriptorBuilder.java index 8d5d68d93335..658ad0641a55 100644 --- a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestTableDescriptorBuilder.java +++ b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestTableDescriptorBuilder.java @@ -161,6 +161,7 @@ public void testSetListRemoveCP() throws Exception { /** * 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