HBASE-19463 Make CPEnv#getConnection return a facade that throws Unsupported if CP calls #close

hbase-server/src/main/java/org/apache/hadoop/hbase/SharedConnection.java

@@ -0,0 +1,93 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase;
+
+import java.io.IOException;
+import java.util.concurrent.ExecutorService;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.client.Admin;
+import org.apache.hadoop.hbase.client.BufferedMutator;
+import org.apache.hadoop.hbase.client.BufferedMutatorParams;
+import org.apache.hadoop.hbase.client.Connection;
+import org.apache.hadoop.hbase.client.RegionLocator;
+import org.apache.hadoop.hbase.client.TableBuilder;
+import org.apache.yetus.audience.InterfaceAudience;
+
+/**
+ * Wraps a Connection to make it can't be closed or aborted.
+ */
+@InterfaceAudience.Private
+public class SharedConnection implements Connection {
+
+  private final Connection conn;
+
+  public SharedConnection(Connection conn) {
+    this.conn = conn;
+  }
+
+  @Override
+  public void close() throws IOException {
+    throw new UnsupportedOperationException("Shared connection");
+  }
+
+  @Override
+  public boolean isClosed() {
+    return this.conn.isClosed();
+  }
+
+  @Override
+  public void abort(String why, Throwable e) {
+    throw new UnsupportedOperationException("Shared connection");
+  }
+
+  @Override
+  public boolean isAborted() {
+    return this.conn.isAborted();
+  }
+
+  @Override
+  public Configuration getConfiguration() {
+    return this.conn.getConfiguration();
+  }
+
+  @Override
+  public BufferedMutator getBufferedMutator(TableName tableName) throws IOException {
+    return this.conn.getBufferedMutator(tableName);
+  }
+
+  @Override
+  public BufferedMutator getBufferedMutator(BufferedMutatorParams params) throws IOException {
+    return this.conn.getBufferedMutator(params);
+  }
+
+  @Override
+  public RegionLocator getRegionLocator(TableName tableName) throws IOException {
+    return this.conn.getRegionLocator(tableName);
+  }
+
+  @Override
+  public Admin getAdmin() throws IOException {
+    return this.conn.getAdmin();
+  }
+
+  @Override
+  public TableBuilder getTableBuilder(TableName tableName, ExecutorService pool) {
+    return this.conn.getTableBuilder(tableName, pool);
+  }
+}

hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterCoprocessorEnvironment.java

@@ -39,10 +39,11 @@ public interface MasterCoprocessorEnvironment extends CoprocessorEnvironment<Mas
   ServerName getServerName();
 
   /**
-   * Returns the hosts' Connection to the Cluster.
+   * Returns the hosts' Connection to the Cluster. <b>Do not close! This is a shared connection
+   * with the hosting server. Throws {@link UnsupportedOperationException} if you try to close
+   * or abort it</b>.
    *
-   * <b>Do not close! Doing so will buckle the hosting server as it depends on its
-   * Connection to function</b>. For light-weight usage only. Heavy-duty usage will pull down
+   * For light-weight usage only. Heavy-duty usage will pull down
    * the hosting RegionServer responsiveness as well as that of other Coprocessors making use of
    * this Connection. Use to create table on start or to do administrative operations. Coprocessors
    * should create their own Connections if heavy usage to avoid impinging on hosting Server

hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/RegionCoprocessorEnvironment.java

@@ -58,10 +58,11 @@ public interface RegionCoprocessorEnvironment extends CoprocessorEnvironment<Reg
   ServerName getServerName();
 
   /**
-   * Returns the hosts' Connection to the Cluster.
+   * Returns the hosts' Connection to the Cluster. <b>Do not close! This is a shared connection
+   * with the hosting server. Throws {@link UnsupportedOperationException} if you try to close
+   * or abort it</b>.
    *
-   * <b>Do not close! Doing so will buckle the hosting server as it depends on its
-   * Connection to function</b>. For light-weight usage only. Heavy-duty usage will pull down
+   * For light-weight usage only. Heavy-duty usage will pull down
    * the hosting RegionServer responsiveness as well as that of other Coprocessors making use of
    * this Connection. Use to create table on start or to do administrative operations. Coprocessors
    * should create their own Connections if heavy usage to avoid impinging on hosting Server

hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/RegionServerCoprocessorEnvironment.java

@@ -45,10 +45,11 @@ public interface RegionServerCoprocessorEnvironment
   OnlineRegions getOnlineRegions();
 
   /**
-   * Returns the hosts' Connection to the Cluster.
+   * Returns the hosts' Connection to the Cluster. <b>Do not close! This is a shared connection
+   * with the hosting server. Throws {@link UnsupportedOperationException} if you try to close
+   * or abort it</b>.
    *
-   * <b>Do not close! Doing so will buckle the hosting server as it depends on its
-   * Connection to function</b>. For light-weight usage only. Heavy-duty usage will pull down
+   * For light-weight usage only. Heavy-duty usage will pull down
    * the hosting RegionServer responsiveness as well as that of other Coprocessors making use of
    * this Connection. Use to create table on start or to do administrative operations. Coprocessors
    * should create their own Connections if heavy usage to avoid impinging on hosting Server

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

@@ -30,6 +30,7 @@ import org.apache.hadoop.hbase.ClusterStatus;
 import org.apache.hadoop.hbase.MetaMutationAnnotation;
 import org.apache.hadoop.hbase.NamespaceDescriptor;
 import org.apache.hadoop.hbase.ServerName;
+import org.apache.hadoop.hbase.SharedConnection;
 import org.apache.hadoop.hbase.TableName;
 import org.apache.hadoop.hbase.client.Connection;
 import org.apache.hadoop.hbase.client.MasterSwitchType;
@@ -99,7 +100,7 @@ public class MasterCoprocessorHost
 
     @Override
     public Connection getConnection() {
-      return this.services.getConnection();
+      return new SharedConnection(this.services.getConnection());
     }
 
     @Override

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

@@ -46,6 +46,7 @@ import org.apache.hadoop.hbase.ExtendedCellBuilderFactory;
 import org.apache.hadoop.hbase.HBaseConfiguration;
 import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.ServerName;
+import org.apache.hadoop.hbase.SharedConnection;
 import org.apache.hadoop.hbase.client.Append;
 import org.apache.hadoop.hbase.client.Connection;
 import org.apache.hadoop.hbase.client.Delete;
@@ -148,7 +149,7 @@ public class RegionCoprocessorHost
     @Override
     public Connection getConnection() {
       // Mocks may have services as null at test time.
-      return services != null ? services.getConnection() : null;
+      return services != null ? new SharedConnection(services.getConnection()) : null;
     }
 
     @Override

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

@@ -25,6 +25,7 @@ import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.ServerName;
+import org.apache.hadoop.hbase.SharedConnection;
 import org.apache.hadoop.hbase.client.Connection;
 import org.apache.hadoop.hbase.coprocessor.BaseEnvironment;
 import org.apache.hadoop.hbase.coprocessor.CoprocessorHost;
@@ -238,7 +239,7 @@ public class RegionServerCoprocessorHost extends
 
     @Override
     public Connection getConnection() {
-      return this.services.getConnection();
+      return new SharedConnection(this.services.getConnection());
     }
 
     @Override

hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoprocessorShortCircuitRPC.java

@@ -21,6 +21,7 @@ import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.CategoryBasedTimeout;
 import org.apache.hadoop.hbase.CoprocessorEnvironment;
 import org.apache.hadoop.hbase.HBaseTestingUtility;
+import org.apache.hadoop.hbase.SharedConnection;
 import org.apache.hadoop.hbase.client.Connection;
 import org.apache.hadoop.hbase.client.ConnectionUtils;
 import org.apache.hadoop.hbase.testclassification.CoprocessorTests;
@@ -73,36 +74,49 @@ public class TestCoprocessorShortCircuitRPC {
   // Three test coprocessors, one of each type that has a Connection in its environment
   // (WALCoprocessor does not).
   public static class TestMasterCoprocessor implements MasterCoprocessor {
-    public TestMasterCoprocessor() {}
+    public TestMasterCoprocessor() {
+    }
 
     @Override
     public void start(CoprocessorEnvironment env) throws IOException {
       // At start, we get base CoprocessorEnvironment Type, not MasterCoprocessorEnvironment,
-      check(((MasterCoprocessorEnvironment)env).getConnection());
+      checkShared(((MasterCoprocessorEnvironment) env).getConnection());
+      checkShortCircuit(
+        ((MasterCoprocessorEnvironment) env).createConnection(env.getConfiguration()));
     }
   }
 
   public static class TestRegionServerCoprocessor implements RegionServerCoprocessor {
-    public TestRegionServerCoprocessor() {}
+    public TestRegionServerCoprocessor() {
+    }
 
     @Override
     public void start(CoprocessorEnvironment env) throws IOException {
       // At start, we get base CoprocessorEnvironment Type, not RegionServerCoprocessorEnvironment,
-      check(((RegionServerCoprocessorEnvironment)env).getConnection());
+      checkShared(((RegionServerCoprocessorEnvironment) env).getConnection());
+      checkShortCircuit(
+        ((RegionServerCoprocessorEnvironment) env).createConnection(env.getConfiguration()));
     }
   }
 
   public static class TestRegionCoprocessor implements RegionCoprocessor {
-    public TestRegionCoprocessor() {}
+    public TestRegionCoprocessor() {
+    }
 
     @Override
     public void start(CoprocessorEnvironment env) throws IOException {
       // At start, we get base CoprocessorEnvironment Type, not RegionCoprocessorEnvironment,
-      check(((RegionCoprocessorEnvironment)env).getConnection());
+      checkShared(((RegionCoprocessorEnvironment) env).getConnection());
+      checkShortCircuit(
+        ((RegionCoprocessorEnvironment) env).createConnection(env.getConfiguration()));
     }
   }
 
-  private static void check(Connection connection) {
+  private static void checkShared(Connection connection) {
+    assertTrue(connection instanceof SharedConnection);
+  }
+
+  private static void checkShortCircuit(Connection connection) {
     assertTrue(connection instanceof ConnectionUtils.ShortCircuitingClusterConnection);
   }