You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by st...@apache.org on 2020/08/18 19:59:04 UTC
[hbase] branch master updated: HBASE-24874 Fix hbase-shell access
to ModifiableTableDescriptor methods (#2268)
This is an automated email from the ASF dual-hosted git repository.
stack pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/master by this push:
new 6789aca HBASE-24874 Fix hbase-shell access to ModifiableTableDescriptor methods (#2268)
6789aca is described below
commit 6789aca9a07b8cb7d9f64e45f2031855a03aedb7
Author: Elliot <el...@apple.com>
AuthorDate: Tue Aug 18 15:58:40 2020 -0400
HBASE-24874 Fix hbase-shell access to ModifiableTableDescriptor methods (#2268)
* HBASE-24874 Fix hbase-shell access to ModifiableTableDescriptor methods
- Fix hbase-shell access in JDK 11 for calls to
TableDescriptorBuilder.toCoprocessorDescriptor and
ModifiableTableDescriptor.toStringTableAttributes.
- Allow coprocessors to be specified using a Ruby hash in the hbase-shell alter
command and replace usage in the help text. The previous String overload of
the alter command will continue to work and is still covered by a unit test,
but will no longer be suggested in the alter command help.
* Update patch
- Add warning over toCoprocessorDescriptor noting the usage by hbase-shell
- Add constants to hbase_constants for coprocessor specification
- Document usage of ModifiableTableDescriptor.toStringTableAttributes
* Convert comment over toCoprocessorDescriptor into docstring
Signed-off-by: Nick Dimiduk <nd...@apache.org>
Signed-off-by: stack <st...@apache.org>
---
.../hbase/client/TableDescriptorBuilder.java | 4 ++
hbase-shell/src/main/ruby/hbase/admin.rb | 65 +++++++++++++++++++---
hbase-shell/src/main/ruby/hbase_constants.rb | 5 ++
hbase-shell/src/main/ruby/shell/commands/alter.rb | 18 +++---
hbase-shell/src/test/ruby/hbase/admin_test.rb | 32 ++++++++++-
5 files changed, 103 insertions(+), 21 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 f5ec540..1328f7d 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
@@ -1628,6 +1628,10 @@ public class TableDescriptorBuilder {
}
}
+ /**
+ * This method is mostly intended for internal use. However, it it also relied on by hbase-shell
+ * for backwards compatibility.
+ */
private static Optional<CoprocessorDescriptor> toCoprocessorDescriptor(String spec) {
Matcher matcher = CP_HTD_ATTR_VALUE_PATTERN.matcher(spec);
if (matcher.matches()) {
diff --git a/hbase-shell/src/main/ruby/hbase/admin.rb b/hbase-shell/src/main/ruby/hbase/admin.rb
index 5392cdf..e3c02e3 100644
--- a/hbase-shell/src/main/ruby/hbase/admin.rb
+++ b/hbase-shell/src/main/ruby/hbase/admin.rb
@@ -26,6 +26,7 @@ java_import org.apache.hadoop.hbase.util.Bytes
java_import org.apache.hadoop.hbase.ServerName
java_import org.apache.hadoop.hbase.TableName
java_import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder
+java_import org.apache.hadoop.hbase.client.CoprocessorDescriptorBuilder
java_import org.apache.hadoop.hbase.client.TableDescriptorBuilder
java_import org.apache.hadoop.hbase.HConstants
@@ -632,7 +633,13 @@ module Hbase
def get_table_attributes(table_name)
tableExists(table_name)
- @admin.getDescriptor(TableName.valueOf(table_name)).toStringTableAttributes
+ td = @admin.getDescriptor TableName.valueOf(table_name)
+ # toStringTableAttributes is a public method, but it is defined on the private class
+ # ModifiableTableDescriptor, so we need reflection to access it in JDK 11+.
+ # TODO Maybe move this to a utility class in the future?
+ method = td.java_class.declared_method :toStringTableAttributes
+ method.accessible = true
+ method.invoke td
end
#----------------------------------------------------------------------------------------------
@@ -710,6 +717,41 @@ module Hbase
end
#----------------------------------------------------------------------------------------------
+ # Use our internal logic to convert from "spec string" format to a coprocessor descriptor
+ #
+ # Provided for backwards shell compatibility
+ #
+ # @param [String] spec_str
+ # @return [ColumnDescriptor]
+ def coprocessor_descriptor_from_spec_str(spec_str)
+ method = TableDescriptorBuilder.java_class.declared_method_smart :toCoprocessorDescriptor
+ method.accessible = true
+ result = method.invoke(nil, spec_str).to_java
+ # unpack java's Optional to be more rubonic
+ return result.isPresent ? result.get : nil
+ end
+
+ #----------------------------------------------------------------------------------------------
+ # Use CoprocessorDescriptorBuilder to convert a Hash to CoprocessorDescriptor
+ #
+ # @param [Hash] spec column descriptor specification
+ # @return [ColumnDescriptor]
+ def coprocessor_descriptor_from_hash(spec)
+ classname = spec[CLASSNAME]
+ raise ArgumentError.new "CLASSNAME must be provided in spec" if classname.nil?
+ jar_path = spec[JAR_PATH]
+ priority = spec[PRIORITY]
+ properties = spec[PROPERTIES]
+
+ builder = CoprocessorDescriptorBuilder.newBuilder classname
+ builder.setJarPath jar_path unless jar_path.nil?
+ builder.setPriority priority unless priority.nil?
+ properties&.each { |k, v| builder.setProperty(k, v.to_s) }
+
+ builder.build
+ end
+
+ #----------------------------------------------------------------------------------------------
# Change table structure or table options
def alter(table_name_str, wait = true, *args)
# Table name should be a string
@@ -817,14 +859,19 @@ module Hbase
k = String.new(key) # prepare to strip
k.strip!
- next unless k =~ /coprocessor/i
- v = String.new(value)
- v.strip!
- # TODO: We should not require user to config the coprocessor with our inner format.
- # This is a roundabout approach, but will be replaced shortly since
- # the setCoprocessorWithSpec method is marked for deprecation.
- coprocessor_descriptors = tdb.build.setCoprocessorWithSpec(v).getCoprocessorDescriptors
- tdb.setCoprocessors(coprocessor_descriptors)
+ # Uses insensitive matching so we can accept lowercase 'coprocessor' for compatibility
+ next unless k =~ /#{COPROCESSOR}/i
+ if value.is_a? String
+ # Specifying a coprocessor by this "spec string" is here for backwards compatibility
+ v = String.new value
+ v.strip!
+ cp = coprocessor_descriptor_from_spec_str v
+ elsif value.is_a? Hash
+ cp = coprocessor_descriptor_from_hash value
+ else
+ raise ArgumentError.new 'coprocessor must be provided as a String or Hash'
+ end
+ tdb.setCoprocessor cp
valid_coproc_keys << key
end
diff --git a/hbase-shell/src/main/ruby/hbase_constants.rb b/hbase-shell/src/main/ruby/hbase_constants.rb
index b1b0eae..1c79007 100644
--- a/hbase-shell/src/main/ruby/hbase_constants.rb
+++ b/hbase-shell/src/main/ruby/hbase_constants.rb
@@ -39,12 +39,14 @@ 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
CONFIG = 'CONFIG'.freeze
CONFIGURATION = org.apache.hadoop.hbase.HConstants::CONFIGURATION
CONSISTENCY = 'CONSISTENCY'.freeze
+ COPROCESSOR = 'COPROCESSOR'.freeze
DATA = 'DATA'.freeze
ENDPOINT_CLASSNAME = 'ENDPOINT_CLASSNAME'.freeze
FILTER = 'FILTER'.freeze
@@ -56,6 +58,7 @@ module HBaseConstants
IN_MEMORY_COMPACTION = org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder::IN_MEMORY_COMPACTION
ISOLATION_LEVEL = 'ISOLATION_LEVEL'.freeze
IS_ROOT = 'IS_ROOT'.freeze
+ JAR_PATH = 'JAR_PATH'.freeze
LIMIT = 'LIMIT'.freeze
LOCALITY_THRESHOLD = 'LOCALITY_THRESHOLD'.freeze
MAXLENGTH = 'MAXLENGTH'.freeze
@@ -69,6 +72,8 @@ module HBaseConstants
NONE = 'NONE'.freeze
NUMREGIONS = 'NUMREGIONS'.freeze
POLICY = 'POLICY'.freeze
+ PRIORITY = 'PRIORITY'.freeze
+ PROPERTIES = 'PROPERTIES'.freeze
RAW = 'RAW'.freeze
READ_TYPE = 'READ_TYPE'.freeze
REGEX = 'REGEX'.freeze
diff --git a/hbase-shell/src/main/ruby/shell/commands/alter.rb b/hbase-shell/src/main/ruby/shell/commands/alter.rb
index e6872fb..456d6d5 100644
--- a/hbase-shell/src/main/ruby/shell/commands/alter.rb
+++ b/hbase-shell/src/main/ruby/shell/commands/alter.rb
@@ -53,19 +53,19 @@ for example, to change the max size of a region to 128MB, do:
hbase> alter 't1', MAX_FILESIZE => '134217728'
-You can add a table coprocessor by setting a table coprocessor attribute:
+You can add a table coprocessor by setting a table coprocessor attribute. Only the CLASSNAME is
+required in the coprocessor specification.
- hbase> alter 't1',
- 'coprocessor'=>'hdfs:///foo.jar|com.foo.FooRegionObserver|1001|arg1=1,arg2=2'
+ hbase> alter 't1', COPROCESSOR => {
+ CLASSNAME => 'org.apache.hadoop.hbase.coprocessor.SimpleRegionObserver',
+ JAR_PATH => 'hdfs:///foo.jar',
+ PRIORITY => 12,
+ PROPERTIES => {'a' => '17' }
+ }
Since you can have multiple coprocessors configured for a table, a
sequence number will be automatically appended to the attribute name
-to uniquely identify it.
-
-The coprocessor attribute must match the pattern below in order for
-the framework to understand how to load the coprocessor classes:
-
- [coprocessor jar file location] | class name | [priority] | [arguments]
+to uniquely identify it. For example, the attribute name might be "coprocessor$1".
You can also set configuration settings specific to this table or column family:
diff --git a/hbase-shell/src/test/ruby/hbase/admin_test.rb b/hbase-shell/src/test/ruby/hbase/admin_test.rb
index 374d9c1..1d26e07 100644
--- a/hbase-shell/src/test/ruby/hbase/admin_test.rb
+++ b/hbase-shell/src/test/ruby/hbase/admin_test.rb
@@ -943,7 +943,7 @@ module Hbase
assert_match(/12345678/, admin.describe(@test_name))
end
- define_test "alter should be able to change coprocessor attributes" do
+ define_test "alter should be able to specify coprocessor attributes with spec string" do
drop_test_table(@test_name)
create_test_table(@test_name)
@@ -956,8 +956,34 @@ module Hbase
assert_no_match(eval("/" + class_name + "/"), admin.describe(@test_name))
assert_no_match(eval("/" + cp_key + "/"), admin.describe(@test_name))
command(:alter, @test_name, 'METHOD' => 'table_att', cp_key => cp_value)
- assert_match(eval("/" + class_name + "/"), admin.describe(@test_name))
- assert_match(eval("/" + cp_key + "\\$(\\d+)/"), admin.describe(@test_name))
+ 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)
+ end
+
+ define_test "alter should be able to change coprocessor attributes with hash" do
+ drop_test_table(@test_name)
+ create_test_table(@test_name)
+
+ cp_key = "coprocessor"
+ class_name = "org.apache.hadoop.hbase.coprocessor.SimpleRegionObserver"
+
+ # eval() is used to convert a string to regex
+ assert_no_match(eval("/" + class_name + "/"), admin.describe(@test_name))
+ assert_no_match(eval("/" + cp_key + "/"), admin.describe(@test_name))
+ command(:alter, @test_name, 'METHOD' => 'table_att', cp_key => {
+ 'CLASSNAME' => class_name,
+ 'PRIORITY' => 15,
+ 'PROPERTIES' => {
+ 'arg1' => 4,
+ 'arg2' => 9,
+ },
+ })
+ describe_text = admin.describe(@test_name)
+ assert_match(eval("/" + class_name + "/"), describe_text)
+ assert_match(eval("/" + cp_key + "\\$(\\d+)/"), describe_text)
+ assert_match(/arg1=4,arg2=9/, describe_text)
end
define_test "alter should be able to remove a table attribute" do