HBASE-12833 [shell] table.rb leaks connections (Solomon Duskis)

Conflicts:
	hbase-shell/src/main/ruby/hbase/hbase.rb

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

@@ -32,15 +32,16 @@ module Hbase
   class Admin
     include HBaseConstants
 
-    def initialize(configuration, formatter)
+    def initialize(admin, formatter)
-      # @admin = org.apache.hadoop.hbase.client.HBaseAdmin.new(configuration)
+      @admin = admin
-      @conn = org.apache.hadoop.hbase.client.ConnectionFactory.createConnection(configuration)
-      @admin = @conn.getAdmin()
       connection = @admin.getConnection()
-      @conf = configuration
       @formatter = formatter
     end
 
+    def close
+      @admin.close
+    end
+
     #----------------------------------------------------------------------------------------------
     # Returns a list of tables in hbase
     def list(regex = ".*")
@@ -196,7 +197,8 @@ module Hbase
     #----------------------------------------------------------------------------------------------
     # Returns ZooKeeper status dump
     def zk_dump
-      @zk_wrapper = org.apache.hadoop.hbase.zookeeper.ZooKeeperWatcher.new(@conf,
+      @zk_wrapper = org.apache.hadoop.hbase.zookeeper.ZooKeeperWatcher.new(
+        @admin.getConfiguration(),
        "admin",
         nil)
       zk = @zk_wrapper.getRecoverableZooKeeper().getZooKeeper()
@@ -397,10 +399,14 @@ module Hbase
     #----------------------------------------------------------------------------------------------
     # Truncates table while maintaing region boundaries (deletes all records by recreating the table)
     def truncate_preserve(table_name, conf = @conf)
-      h_table = @conn.getTable(table_name)
+      h_table = @conn.getTable(table_name)      
-      splits = h_table.getRegionLocations().keys().map{|i| Bytes.toStringBinary(i.getStartKey)}.delete_if{|k| k == ""}.to_java :String
+      locator = @conn.getRegionLocator(table_name)
-      splits = org.apache.hadoop.hbase.util.Bytes.toBinaryByteArrays(splits)
+      splits = locator.getAllRegionLocations().
-      table_description = h_table.getTableDescriptor()
+          map{|i| Bytes.toString(i.getRegionInfo().getStartKey)}.
+          delete_if{|k| k == ""}.to_java :String
+      locator.close()
+
+      table_description = @admin.getTableDescriptor(table_name)
       yield 'Disabling table...' if block_given?
       disable(table_name)
 

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

@@ -38,15 +38,17 @@ module Hbase
         configuration.setInt("hbase.client.retries.number", 7)
         configuration.setInt("hbase.ipc.client.connect.max.retries", 3)
       end
+      @connection = org.apache.hadoop.hbase.client.ConnectionFactory.createConnection(
+          self.configuration)
     end
 
     def admin(formatter)
-      ::Hbase::Admin.new(configuration, formatter)
+      ::Hbase::Admin.new(@connection.getAdmin, formatter)
     end
 
     # Create new one each time
     def table(table, shell)
-      ::Hbase::Table.new(configuration, table, shell)
+      ::Hbase::Table.new(@connection.getTable(table), shell)
     end
 
     def replication_admin(formatter)
@@ -60,5 +62,9 @@ module Hbase
     def visibility_labels_admin(formatter)
       ::Hbase::VisibilityLabelsAdmin.new(configuration, formatter)
     end
+
+    def shutdown
+      @connection.close
+    end
   end
 end

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

@@ -111,23 +111,17 @@ EOF
     # let external objects read the table name
     attr_reader :name
 
-    def initialize(configuration, table_name, shell)
+    def initialize(table, shell)
-      if @@thread_pool then
+      @table = table
-        @connection = org.apache.hadoop.hbase.client.ConnectionFactory.createConnection(
+      @name = @table.getName().getNameAsString()
-          configuration, @@thread_pool)
-        @table = @connection.getTable(org.apache.hadoop.hbase.TableName.valueOf(table_name))
-      else
-        @connection = org.apache.hadoop.hbase.client.ConnectionFactory.createConnection(
-          configuration)
-        @table = @connection.getTable(org.apache.hadoop.hbase.TableName.valueOf(table_name))
-        @@thread_pool = @table.getPool()
-      end
-      
-      @name = table_name
       @shell = shell
       @converters = Hash.new()
     end
 
+    def close()
+      @table.close()
+    end
+
     # Note the below methods are prefixed with '_' to hide them from the average user, as
     # they will be much less likely to tab complete to the 'dangerous' internal method
     #----------------------------------------------------------------------------------------------

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

@@ -36,6 +36,10 @@ module Hbase
       create_test_table(@test_name)
     end
 
+    def teardown
+      shutdown
+    end
+
     define_test "exists? should return true when a table exists" do
       assert(admin.exists?('hbase:meta'))
     end
@@ -69,6 +73,10 @@ module Hbase
       @create_test_name = 'hbase_create_table_test_table'
     end
 
+    def teardown
+      shutdown
+    end
+
     define_test "list should return a list of tables" do
       assert(admin.list.member?(@test_name))
     end
@@ -241,6 +249,10 @@ module Hbase
       create_test_table(@test_name)
     end
 
+    def teardown
+      shutdown
+    end
+
     #-------------------------------------------------------------------------------
 
     define_test "alter should fail with non-string table names" do
@@ -342,6 +354,7 @@ module Hbase
 
       table = table(@test_name)
       assert_not_equal(nil, table)
+      table.close
     end
   end
 end

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

@@ -40,6 +40,10 @@ module Hbase
       @create_test_name = 'hbase_create_table_test_table'
     end
 
+    def teardown
+      shutdown
+    end
+
     define_test "Revoke should rid access rights appropriately" do
       drop_test_table(@test_name)
       create_test_table(@test_name)

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

@@ -29,9 +29,13 @@ module Hbase
       setup_hbase
     end
 
+    def teardown
+      shutdown
+    end
+
     define_test "Hbase::Table constructor should not fail for existent tables" do
       assert_nothing_raised do
-        table('hbase:meta')
+        table('hbase:meta').close()
       end
     end
   end
@@ -48,6 +52,11 @@ module Hbase
       @test_table = table(@test_name)
     end
 
+    def tearDown
+      @test_table.close()
+      shutdown
+    end
+
     define_test "is_meta_table? method should return true for the meta table" do
       assert(table('hbase:meta').is_meta_table?)
     end
@@ -113,6 +122,11 @@ module Hbase
       @test_table.put(105, "x:a", "4")
     end
 
+    def teardown
+      @test_table.close
+      shutdown
+    end
+
     define_test "put should work without timestamp" do
       @test_table.put("123", "x:a", "1")
     end
@@ -203,7 +217,11 @@ module Hbase
       
       @test_table.put(3, "x:a", 21, {ATTRIBUTES=>{'mykey'=>'myvalue'}})
       @test_table.put(3, "x:b", 22, @test_ts, {ATTRIBUTES=>{'mykey'=>'myvalue'}})
+    end
 
+    def teardown
+      @test_table.close
+      shutdown
     end
 
     define_test "count should work w/o a block passed" do

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

@@ -38,6 +38,11 @@ module Hbase
       create_test_table(@test_name)
     end
 
+    def teardown
+      @test_table.close
+      shutdown
+    end
+
     define_test "Labels should be created as specified" do
       label = 'TEST_LABELS'
       count = table('hbase:labels')._count_internal

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

@@ -47,6 +47,10 @@ module Hbase
       hbase = ::Hbase::Hbase.new($TEST_CLUSTER.getConfiguration)
       @shell = ::Shell::Shell.new(hbase, formatter)
     end
+    
+    def shutdown
+      @shell.hbase.shutdown
+    end
 
     def table(table)
       @shell.hbase_table(table)