From 8be26d570d4e1410c49d9f3ad440ef7978498f2d Mon Sep 17 00:00:00 2001 From: "Tak Lon (Stephen) Wu" Date: Tue, 30 Nov 2021 17:16:52 -0800 Subject: [PATCH 1/5] HBASE-26524 Support remove coprocessor by class name via alter table command --- .../hbase/client/TableDescriptorBuilder.java | 8 ++++++-- .../client/TestTableDescriptorBuilder.java | 14 ++++++++++++++ hbase-shell/src/main/ruby/hbase/admin.rb | 11 +++++++++++ .../src/main/ruby/shell/commands/alter.rb | 6 ++++++ hbase-shell/src/test/ruby/hbase/admin_test.rb | 19 +++++++++++++++++++ 5 files changed, 56 insertions(+), 2 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 b33388f4e0a3..9663fcae6832 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 @@ -436,7 +436,7 @@ public TableDescriptorBuilder removeColumnFamily(final byte[] name) { return this; } - public TableDescriptorBuilder removeCoprocessor(String className) { + public TableDescriptorBuilder removeCoprocessor(String className) throws IOException { desc.removeCoprocessor(className); return this; } @@ -1534,7 +1534,7 @@ public List getCoprocessorDescriptors() { * * @param className Class name of the co-processor */ - public void removeCoprocessor(String className) { + public void removeCoprocessor(String className) throws IOException { Bytes match = null; Matcher keyMatcher; Matcher valueMatcher; @@ -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 IOException(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 508e9bd0776d..0ffb252f9a25 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 @@ -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 IOException + */ + @Test(expected = IOException.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 0d02dbaaa1d8..53d262a14f0f 100644 --- a/hbase-shell/src/main/ruby/hbase/admin.rb +++ b/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 diff --git a/hbase-shell/src/main/ruby/shell/commands/alter.rb b/hbase-shell/src/main/ruby/shell/commands/alter.rb index b06ada00248e..b2623b81d923 100644 --- a/hbase-shell/src/main/ruby/shell/commands/alter.rb +++ b/hbase-shell/src/main/ruby/shell/commands/alter.rb @@ -82,6 +82,12 @@ def help hbase> alter 't1', METHOD => 'table_att_unset', NAME => 'coprocessor$1' +To remove coprocessor from the table-scope attribute via 'table_att_unset', you can also use +'table_remove_coprocessor': + + hbase> alter 't1', METHOD => 'table_remove_coprocessor', CLASSNAME => + 'org.apache.hadoop.hbase.coprocessor.SimpleRegionObserver' + 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 957c01895ba3..99fb27e0f769 100644 --- a/hbase-shell/src/test/ruby/hbase/admin_test.rb +++ b/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) From 0fb6499e578ddb7bc986e78fe71310989cd3b98c Mon Sep 17 00:00:00 2001 From: "Tak Lon (Stephen) Wu" Date: Tue, 30 Nov 2021 21:31:24 -0800 Subject: [PATCH 2/5] update a description --- hbase-shell/src/main/ruby/shell/commands/alter.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hbase-shell/src/main/ruby/shell/commands/alter.rb b/hbase-shell/src/main/ruby/shell/commands/alter.rb index b2623b81d923..fc74cd590c44 100644 --- a/hbase-shell/src/main/ruby/shell/commands/alter.rb +++ b/hbase-shell/src/main/ruby/shell/commands/alter.rb @@ -82,8 +82,8 @@ def help hbase> alter 't1', METHOD => 'table_att_unset', NAME => 'coprocessor$1' -To remove coprocessor from the table-scope attribute via 'table_att_unset', you can also use -'table_remove_coprocessor': +Other than removing coprocessor from the table-scope attribute via 'table_att_unset', you can also +use 'table_remove_coprocessor': hbase> alter 't1', METHOD => 'table_remove_coprocessor', CLASSNAME => 'org.apache.hadoop.hbase.coprocessor.SimpleRegionObserver' From 79398084770c1fd06bc219b16f97263bbf8c0e84 Mon Sep 17 00:00:00 2001 From: "Tak Lon (Stephen) Wu" Date: Wed, 1 Dec 2021 08:55:15 -0800 Subject: [PATCH 3/5] update using array for removing multiple cps --- hbase-shell/src/main/ruby/shell/commands/alter.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/hbase-shell/src/main/ruby/shell/commands/alter.rb b/hbase-shell/src/main/ruby/shell/commands/alter.rb index fc74cd590c44..941b1ab62a0b 100644 --- a/hbase-shell/src/main/ruby/shell/commands/alter.rb +++ b/hbase-shell/src/main/ruby/shell/commands/alter.rb @@ -88,6 +88,11 @@ def help 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} From a70f1198a3dbd7d0af0852e5bf241a0cd7f26e02 Mon Sep 17 00:00:00 2001 From: "Tak Lon (Stephen) Wu" Date: Wed, 1 Dec 2021 09:00:55 -0800 Subject: [PATCH 4/5] fix comment in the help page --- hbase-shell/src/main/ruby/shell/commands/alter.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/hbase-shell/src/main/ruby/shell/commands/alter.rb b/hbase-shell/src/main/ruby/shell/commands/alter.rb index 941b1ab62a0b..ad0cb5a5a49b 100644 --- a/hbase-shell/src/main/ruby/shell/commands/alter.rb +++ b/hbase-shell/src/main/ruby/shell/commands/alter.rb @@ -83,12 +83,13 @@ 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': +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 +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'] From f1ebec52f0b52370f3aa3cfa322664d7645ecda7 Mon Sep 17 00:00:00 2001 From: "Tak Lon (Stephen) Wu" Date: Wed, 1 Dec 2021 10:48:59 -0800 Subject: [PATCH 5/5] change IOException to IllegalArgumentException --- .../apache/hadoop/hbase/client/TableDescriptorBuilder.java | 6 +++--- .../hadoop/hbase/client/TestTableDescriptorBuilder.java | 4 ++-- 2 files changed, 5 insertions(+), 5 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 9663fcae6832..0128458645cb 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 @@ -436,7 +436,7 @@ public TableDescriptorBuilder removeColumnFamily(final byte[] name) { return this; } - public TableDescriptorBuilder removeCoprocessor(String className) throws IOException { + public TableDescriptorBuilder removeCoprocessor(String className) { desc.removeCoprocessor(className); return this; } @@ -1534,7 +1534,7 @@ public List getCoprocessorDescriptors() { * * @param className Class name of the co-processor */ - public void removeCoprocessor(String className) throws IOException { + public void removeCoprocessor(String className) { Bytes match = null; Matcher keyMatcher; Matcher valueMatcher; @@ -1562,7 +1562,7 @@ public void removeCoprocessor(String className) throws IOException { if (match != null) { ModifyableTableDescriptor.this.removeValue(match); } else { - throw new IOException(String + 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 0ffb252f9a25..a9b7cd99fe37 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 @@ -150,9 +150,9 @@ 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 IOException + * @throws Exception if removing a coprocessor fails other than IllegalArgumentException */ - @Test(expected = IOException.class) + @Test(expected = IllegalArgumentException.class) public void testRemoveNonExistingCoprocessor() throws Exception { String className = "org.apache.hadoop.hbase.coprocessor.SimpleRegionObserver"; TableDescriptor desc = TableDescriptorBuilder