HBASE-12518 Task 4 polish. Remove CM#{get,delete}Connection

Remove deleteConnection from everywhere except the two tests that ensure
it does the right thing and from HCM and CM themselves.

Undoes deleteConnection from tests and from the web ui

hbase-server/src/main/jamon/org/apache/hadoop/hbase/tmpl/master/MasterStatusTmpl.jamon

@@ -18,7 +18,6 @@ limitations under the License.
 </%doc>
 <%args>
 HMaster master;
-HBaseAdmin admin;
 Map<String, Integer> frags = null;
 ServerName metaLocation = null;
 List<ServerName> servers = null;
@@ -42,7 +41,7 @@ org.apache.hadoop.hbase.HConstants;
 org.apache.hadoop.hbase.NamespaceDescriptor;
 org.apache.hadoop.hbase.ServerLoad;
 org.apache.hadoop.hbase.ServerName;
-org.apache.hadoop.hbase.client.HBaseAdmin;
+org.apache.hadoop.hbase.client.Admin;
 org.apache.hadoop.hbase.client.HConnectionManager;
 org.apache.hadoop.hbase.HTableDescriptor;
 org.apache.hadoop.hbase.HBaseConfiguration;
@@ -307,8 +306,11 @@ AssignmentManager assignmentManager = master.getAssignmentManager();
 
 <%def catalogTables>
 <%java>
- HTableDescriptor[] sysTables = master.isInitialized() ? admin.listTableDescriptorsByNamespace(
-   NamespaceDescriptor.SYSTEM_NAMESPACE_NAME_STR) : null;
+ HTableDescriptor[] sysTables = null;
+ try (Admin admin = master.getConnection().getAdmin()) {
+   sysTables = master.isInitialized() ? admin.listTableDescriptorsByNamespace(
+     NamespaceDescriptor.SYSTEM_NAMESPACE_NAME_STR) : null;
+ }
 </%java>
 <%if (sysTables != null && sysTables.length > 0)%>
 <table class="table table-striped">
@@ -347,7 +349,10 @@ AssignmentManager assignmentManager = master.getAssignmentManager();
 
 <%def userTables>
 <%java>
-   HTableDescriptor[] tables = master.isInitialized() ? admin.listTables() : null;
+   HTableDescriptor[] tables = null; 
+   try (Admin admin = master.getConnection().getAdmin()) {
+     tables = master.isInitialized() ? admin.listTables() : null;
+   }
 </%java>
 <%if (tables != null && tables.length > 0)%>
 <table class="table table-striped">
@@ -379,7 +384,10 @@ AssignmentManager assignmentManager = master.getAssignmentManager();
 
 <%def userSnapshots>
 <%java>
-   List<SnapshotDescription> snapshots = master.isInitialized() ? admin.listSnapshots() : null;
+   List<SnapshotDescription> snapshots = null;
+   try (Admin admin = master.getConnection().getAdmin()) {
+     snapshots = master.isInitialized() ? admin.listSnapshots() : null;
+   }
 </%java>
 <%if (snapshots != null && snapshots.size() > 0)%>
 <table class="table table-striped">
@@ -436,7 +444,4 @@ AssignmentManager assignmentManager = master.getAssignmentManager();
     </tr>
 </table>
 </%if>
-<%java>
-   HConnectionManager.deleteConnection(admin.getConfiguration());
-</%java>
 </%def>

hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterStatusServlet.java

@@ -53,7 +53,6 @@ public class MasterStatusServlet extends HttpServlet {
     response.setContentType("text/html");
 
     Configuration conf = master.getConfiguration();
-    HBaseAdmin admin = new HBaseAdmin(conf);
 
     Map<String, Integer> frags = getFragmentationInfo(master, conf);
     ServerName metaLocation = null;
@@ -80,8 +79,7 @@ public class MasterStatusServlet extends HttpServlet {
       tmpl.setFilter(request.getParameter("filter"));
     if (request.getParameter("format") != null)
       tmpl.setFormat(request.getParameter("format"));
-    tmpl.render(response.getWriter(),
-          master, admin);
+    tmpl.render(response.getWriter(), master);
   }
 
   private ServerName getMetaLocationOrNull(HMaster master) {

hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSStatusServlet.java

@@ -34,10 +34,8 @@ public class RSStatusServlet extends HttpServlet {
 
   @Override
   protected void doGet(HttpServletRequest req, HttpServletResponse resp)
-      throws ServletException, IOException
-    {
-    HRegionServer hrs = (HRegionServer)getServletContext().getAttribute(
-        HRegionServer.REGIONSERVER);
+  throws ServletException, IOException {
+    HRegionServer hrs = (HRegionServer)getServletContext().getAttribute(HRegionServer.REGIONSERVER);
     assert hrs != null : "No RS in context!";
 
     resp.setContentType("text/html");
@@ -59,4 +57,4 @@ public class RSStatusServlet extends HttpServlet {
       tmpl.setBcv(req.getParameter("bcv"));
     tmpl.render(resp.getWriter(), hrs);
   }
-}
+}

hbase-server/src/main/resources/hbase-webapps/master/snapshot.jsp

@@ -20,7 +20,7 @@
 <%@ page contentType="text/html;charset=UTF-8"
   import="java.util.Date"
   import="org.apache.hadoop.conf.Configuration"
-  import="org.apache.hadoop.hbase.client.HBaseAdmin"
+  import="org.apache.hadoop.hbase.client.Admin"
   import="org.apache.hadoop.hbase.client.HConnectionManager"
   import="org.apache.hadoop.hbase.master.HMaster"
   import="org.apache.hadoop.hbase.snapshot.SnapshotInfo"
@@ -31,18 +31,19 @@
 <%
   HMaster master = (HMaster)getServletContext().getAttribute(HMaster.MASTER);
   Configuration conf = master.getConfiguration();
-  HBaseAdmin hbadmin = new HBaseAdmin(conf);
   boolean readOnly = conf.getBoolean("hbase.master.ui.readonly", false);
   String snapshotName = request.getParameter("name");
   SnapshotDescription snapshot = null;
   SnapshotInfo.SnapshotStats stats = null;
   TableName snapshotTable = null;
-  for (SnapshotDescription snapshotDesc: hbadmin.listSnapshots()) {
-    if (snapshotName.equals(snapshotDesc.getName())) {
-      snapshot = snapshotDesc;
-      stats = SnapshotInfo.getSnapshotStats(conf, snapshot);
-      snapshotTable = TableName.valueOf(snapshot.getTable());
-      break;
+  try (Admin admin = master.getConnection().getAdmin()) {
+    for (SnapshotDescription snapshotDesc: admin.listSnapshots()) {
+      if (snapshotName.equals(snapshotDesc.getName())) {
+        snapshot = snapshotDesc;
+        stats = SnapshotInfo.getSnapshotStats(conf, snapshot);
+        snapshotTable = TableName.valueOf(snapshot.getTable());
+        break;
+      }
     }
   }
 
@@ -112,15 +113,17 @@
     </div>
     <p><hr><p>
 <%
-  if (action.equals("restore")) {
-    hbadmin.restoreSnapshot(snapshotName);
-    %> Restore Snapshot request accepted. <%
-  } else if (action.equals("clone")) {
-    if (cloneName != null && cloneName.length() > 0) {
-      hbadmin.cloneSnapshot(snapshotName, cloneName);
-      %> Clone from Snapshot request accepted. <%
-    } else {
-      %> Clone from Snapshot request failed, No table name specified. <%
+  try (Admin admin = master.getConnection().getAdmin()) {
+    if (action.equals("restore")) {
+      admin.restoreSnapshot(snapshotName);
+      %> Restore Snapshot request accepted. <%
+    } else if (action.equals("clone")) {
+      if (cloneName != null && cloneName.length() > 0) {
+        admin.cloneSnapshot(snapshotName, TableName.valueOf(cloneName));
+        %> Clone from Snapshot request accepted. <%
+      } else {
+        %> Clone from Snapshot request failed, No table name specified. <%
+      }
     }
   }
 %>
@@ -189,8 +192,6 @@
   <% } %>
 <%
   } // end else
-
-HConnectionManager.deleteConnection(hbadmin.getConfiguration());
 %>
 
 

hbase-server/src/main/resources/hbase-webapps/master/table.jsp

@@ -23,7 +23,7 @@
   import="java.util.Map"
   import="org.apache.hadoop.conf.Configuration"
   import="org.apache.hadoop.hbase.client.HTable"
-  import="org.apache.hadoop.hbase.client.HBaseAdmin"
+  import="org.apache.hadoop.hbase.client.Admin"
   import="org.apache.hadoop.hbase.client.HConnectionManager"
   import="org.apache.hadoop.hbase.HRegionInfo"
   import="org.apache.hadoop.hbase.ServerName"
@@ -41,7 +41,6 @@
 <%
   HMaster master = (HMaster)getServletContext().getAttribute(HMaster.MASTER);
   Configuration conf = master.getConfiguration();
-  HBaseAdmin hbadmin = new HBaseAdmin(conf);
   MetaTableLocator metaTableLocator = new MetaTableLocator();
   String fqtn = request.getParameter("name");
   HTable table = new HTable(conf, fqtn);
@@ -125,21 +124,23 @@
         </div>
 <p><hr><p>
 <%
-  if (action.equals("split")) {
-    if (key != null && key.length() > 0) {
-      hbadmin.split(key);
-    } else {
-      hbadmin.split(fqtn);
-    }
+  try (Admin admin = master.getConnection().getAdmin()) {
+    if (action.equals("split")) {
+      if (key != null && key.length() > 0) {
+        admin.splitRegion(Bytes.toBytes(key));
+      } else {
+        admin.split(TableName.valueOf(fqtn));
+      }
     
     %> Split request accepted. <%
-  } else if (action.equals("compact")) {
-    if (key != null && key.length() > 0) {
-      hbadmin.compact(key);
-    } else {
-      hbadmin.compact(fqtn);
-    }
+    } else if (action.equals("compact")) {
+      if (key != null && key.length() > 0) {
+        admin.compactRegion(Bytes.toBytes(key));
+      } else {
+        admin.compact(TableName.valueOf(fqtn));
+      }
     %> Compact request accepted. <%
+    }
   }
 %>
 <p>Go <a href="javascript:history.back()">Back</a>, or wait for the redirect.
@@ -220,6 +221,7 @@
 <%  } %>
 </table>
 <%} else {
+  Admin admin = master.getConnection().getAdmin();
   try { %>
 <h2>Table Attributes</h2>
 <table class="table table-striped">
@@ -230,7 +232,7 @@
   </tr>
   <tr>
       <td>Enabled</td>
-      <td><%= hbadmin.isTableEnabled(table.getTableName()) %></td>
+      <td><%= admin.isTableEnabled(table.getName()) %></td>
       <td>Is the table enabled</td>
   </tr>
   <tr>
@@ -238,7 +240,7 @@
       <td>
 <%
   try {
-    CompactionState compactionState = hbadmin.getCompactionState(table.getTableName());
+    CompactionState compactionState = admin.getCompactionState(table.getName());
 %>
 <%= compactionState %>
 <%
@@ -335,10 +337,10 @@
 <% }
 } catch(Exception ex) {
   ex.printStackTrace(System.err);
+} finally {
+  admin.close();
 }
 } // end else
-
-HConnectionManager.deleteConnection(hbadmin.getConfiguration());
 %>
 
 

hbase-server/src/test/java/org/apache/hadoop/hbase/TestMetaTableAccessorNoCluster.java

@@ -30,8 +30,6 @@ import java.util.NavigableMap;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.apache.hadoop.hbase.client.ClusterConnection;
-import org.apache.hadoop.hbase.client.HConnection;
-import org.apache.hadoop.hbase.client.HConnectionManager;
 import org.apache.hadoop.hbase.client.HConnectionTestingUtility;
 import org.apache.hadoop.hbase.client.Result;
 import org.apache.hadoop.hbase.ipc.PayloadCarryingRpcController;
@@ -130,7 +128,7 @@ public class TestMetaTableAccessorNoCluster {
     // This is a servername we use in a few places below.
     ServerName sn = ServerName.valueOf("example.com", 1234, System.currentTimeMillis());
 
-    HConnection connection;
+    ClusterConnection connection = null;
     try {
       // Mock an ClientProtocol. Our mock implementation will fail a few
       // times when we go to open a scanner.
@@ -206,7 +204,7 @@ public class TestMetaTableAccessorNoCluster {
       Mockito.verify(implementation, Mockito.times(6)).
         scan((RpcController)Mockito.any(), (ScanRequest)Mockito.any());
     } finally {
-      HConnectionManager.deleteConnection(UTIL.getConfiguration());
+      if (connection != null && !connection.isClosed()) connection.close();
       zkw.close();
     }
   }

hbase-server/src/test/java/org/apache/hadoop/hbase/TestMetaTableLocator.java

@@ -29,8 +29,7 @@ import java.net.ConnectException;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.apache.hadoop.conf.Configuration;
-import org.apache.hadoop.hbase.client.HConnection;
-import org.apache.hadoop.hbase.client.HConnectionManager;
+import org.apache.hadoop.hbase.client.ClusterConnection;
 import org.apache.hadoop.hbase.client.HConnectionTestingUtility;
 import org.apache.hadoop.hbase.ipc.ServerNotRunningYetException;
 import org.apache.hadoop.hbase.master.RegionState;
@@ -103,9 +102,6 @@ public class TestMetaTableLocator {
       LOG.warn("Unable to delete hbase:meta location", e);
     }
 
-    // Clear out our doctored connection or could mess up subsequent tests.
-    HConnectionManager.deleteConnection(UTIL.getConfiguration());
-
     this.watcher.close();
   }
 
@@ -184,7 +180,8 @@ public class TestMetaTableLocator {
     // Mock an ClientProtocol.
     final ClientProtos.ClientService.BlockingInterface implementation =
       Mockito.mock(ClientProtos.ClientService.BlockingInterface.class);
-    HConnection connection = mockConnection(null, implementation);
+ 
+    ClusterConnection connection = mockConnection(null, implementation);
 
     // If a 'get' is called on mocked interface, throw connection refused.
     Mockito.when(implementation.get((RpcController) Mockito.any(), (GetRequest) Mockito.any())).
@@ -244,14 +241,14 @@ public class TestMetaTableLocator {
   @Test
   public void testVerifyMetaRegionLocationFails()
   throws IOException, InterruptedException, KeeperException, ServiceException {
-    HConnection connection = Mockito.mock(HConnection.class);
+    ClusterConnection connection = Mockito.mock(ClusterConnection.class);
     ServiceException connectException =
       new ServiceException(new ConnectException("Connection refused"));
     final AdminProtos.AdminService.BlockingInterface implementation =
       Mockito.mock(AdminProtos.AdminService.BlockingInterface.class);
     Mockito.when(implementation.getRegionInfo((RpcController)Mockito.any(),
       (GetRegionInfoRequest)Mockito.any())).thenThrow(connectException);
-    Mockito.when(connection.getAdmin(Mockito.any(ServerName.class), Mockito.anyBoolean())).
+    Mockito.when(connection.getAdmin(Mockito.any(ServerName.class))).
       thenReturn(implementation);
 
     ServerName sn = ServerName.valueOf("example.com", 1234, System.currentTimeMillis());
@@ -303,20 +300,17 @@ public class TestMetaTableLocator {
    * and that returns the passed {@link AdminProtos.AdminService.BlockingInterface} instance when
    * {@link HConnection#getAdmin(ServerName)} is called, returns the passed
    * {@link ClientProtos.ClientService.BlockingInterface} instance when
-   * {@link HConnection#getClient(ServerName)} is called (Be sure to call
-   * {@link HConnectionManager#deleteConnection(org.apache.hadoop.conf.Configuration)}
-   * when done with this mocked Connection.
+   * {@link HConnection#getClient(ServerName)} is called.
    * @throws IOException
    */
-  private HConnection mockConnection(final AdminProtos.AdminService.BlockingInterface admin,
+  private ClusterConnection mockConnection(final AdminProtos.AdminService.BlockingInterface admin,
       final ClientProtos.ClientService.BlockingInterface client)
   throws IOException {
-    HConnection connection =
+    ClusterConnection connection =
       HConnectionTestingUtility.getMockedConnection(UTIL.getConfiguration());
     Mockito.doNothing().when(connection).close();
     // Make it so we return any old location when asked.
-    final HRegionLocation anyLocation =
-      new HRegionLocation(HRegionInfo.FIRST_META_REGIONINFO, SN);
+    final HRegionLocation anyLocation = new HRegionLocation(HRegionInfo.FIRST_META_REGIONINFO, SN);
     Mockito.when(connection.getRegionLocation((TableName) Mockito.any(),
         (byte[]) Mockito.any(), Mockito.anyBoolean())).
       thenReturn(anyLocation);

hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java

@@ -297,6 +297,11 @@ public class TestFromClientSide {
      table.close();
    }
 
+   /**
+    * @deprecated Tests deprecated functionality. Remove when we are past 1.0.
+    * @throws Exception
+    */
+   @Deprecated
    @Test
    public void testSharedZooKeeper() throws Exception {
      Configuration newConfig = new Configuration(TEST_UTIL.getConfiguration());

hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestHCM.java

@@ -875,7 +875,9 @@ public class TestHCM {
    * Makes sure that there is no leaking of
    * {@link ConnectionManager.HConnectionImplementation} in the {@link HConnectionManager}
    * class.
+   * @deprecated Tests deprecated functionality.  Remove in 1.0.
    */
+  @Deprecated
   @Test
   public void testConnectionUniqueness() throws Exception {
     int zkmaxconnections = TEST_UTIL.getConfiguration().

hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java

@@ -199,9 +199,6 @@ public class TestCatalogJanitor {
 
     @Override
     public void stop(String why) {
-      if (this.connection != null) {
-        HConnectionManager.deleteConnection(this.connection.getConfiguration());
-      }
     }
   }
 

hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterStatusServlet.java

@@ -54,7 +54,7 @@ import com.google.common.collect.Maps;
  */
 @Category({MasterTests.class,MediumTests.class})
 public class TestMasterStatusServlet {
-  
+
   private HMaster master;
   private Configuration conf;
   private HBaseAdmin admin;
@@ -110,7 +110,7 @@ public class TestMasterStatusServlet {
     Mockito.doReturn(rms).when(master).getRegionServerMetrics();
 
     // Mock admin
-    admin = Mockito.mock(HBaseAdmin.class); 
+    admin = Mockito.mock(HBaseAdmin.class);
   }
 
   private void setupMockTables() throws IOException {
@@ -120,27 +120,25 @@ public class TestMasterStatusServlet {
     };
     Mockito.doReturn(tables).when(admin).listTables();
   }
-  
+
   @Test
   public void testStatusTemplateNoTables() throws IOException {
-    new MasterStatusTmpl().render(new StringWriter(),
-        master, admin);
+    new MasterStatusTmpl().render(new StringWriter(), master);
   }
 
   @Test
   public void testStatusTemplateMetaAvailable() throws IOException {
     setupMockTables();
-    
+
     new MasterStatusTmpl()
       .setMetaLocation(ServerName.valueOf("metaserver:123,12345"))
-      .render(new StringWriter(),
-        master, admin);
+      .render(new StringWriter(), master);
   }
 
   @Test
   public void testStatusTemplateWithServers() throws IOException {
     setupMockTables();
-    
+
     List<ServerName> servers = Lists.newArrayList(
         ServerName.valueOf("rootserver:123,12345"),
         ServerName.valueOf("metaserver:123,12345"));
@@ -154,10 +152,9 @@ public class TestMasterStatusServlet {
       .setMetaLocation(ServerName.valueOf("metaserver:123,12345"))
       .setServers(servers)
       .setDeadServers(deadServers)
-      .render(new StringWriter(),
-        master, admin);
+      .render(new StringWriter(), master);
   }
-  
+
   @Test
   public void testAssignmentManagerTruncatedList() throws IOException {
     AssignmentManager am = Mockito.mock(AssignmentManager.class);
@@ -189,7 +186,7 @@ public class TestMasterStatusServlet {
 
     // Should always include META
     assertTrue(result.contains(HRegionInfo.FIRST_META_REGIONINFO.getEncodedName()));
-    
+
     // Make sure we only see 50 of them
     Matcher matcher = Pattern.compile("CLOSING").matcher(result);
     int count = 0;