You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by GitBox <gi...@apache.org> on 2020/07/29 07:33:04 UTC

[GitHub] [hbase] bharathv commented on a change in pull request #2162: HBASE-24788: Fix the connection leaks in hbase

bharathv commented on a change in pull request #2162:
URL: https://github.com/apache/hbase/pull/2162#discussion_r461865026



##########
File path: hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/TableOutputFormat.java
##########
@@ -155,11 +155,10 @@ public void write(KEY key, Mutation value)
    * @param context  The current task context.
    * @return The newly created writer instance.
    * @throws IOException When creating the writer fails.
-   * @throws InterruptedException When the jobs is cancelled.
    */
   @Override
   public RecordWriter<KEY, Mutation> getRecordWriter(TaskAttemptContext context)
-  throws IOException, InterruptedException {
+  throws IOException {

Review comment:
       nit: Check-style might flag this for bad indentation (needs nested indent)

##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/util/ServerRegionReplicaUtil.java
##########
@@ -161,25 +162,27 @@ public static void setupRegionReplicaReplication(Configuration conf) throws IOEx
     if (!isRegionReplicaReplicationEnabled(conf)) {
       return;
     }
-    Admin admin = ConnectionFactory.createConnection(conf).getAdmin();
-    ReplicationPeerConfig peerConfig = null;
-    try {
-      peerConfig = admin.getReplicationPeerConfig(REGION_REPLICA_REPLICATION_PEER);
-    } catch (ReplicationPeerNotFoundException e) {
-      LOG.warn("Region replica replication peer id=" + REGION_REPLICA_REPLICATION_PEER
-          + " not exist", e);
-    }
-    try {
+
+    try (Connection connection = ConnectionFactory.createConnection(conf);
+      Admin admin = connection.getAdmin()) {
+

Review comment:
       nit: remove extra newline

##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/util/ServerRegionReplicaUtil.java
##########
@@ -161,25 +162,27 @@ public static void setupRegionReplicaReplication(Configuration conf) throws IOEx
     if (!isRegionReplicaReplicationEnabled(conf)) {
       return;
     }
-    Admin admin = ConnectionFactory.createConnection(conf).getAdmin();
-    ReplicationPeerConfig peerConfig = null;
-    try {
-      peerConfig = admin.getReplicationPeerConfig(REGION_REPLICA_REPLICATION_PEER);
-    } catch (ReplicationPeerNotFoundException e) {
-      LOG.warn("Region replica replication peer id=" + REGION_REPLICA_REPLICATION_PEER
-          + " not exist", e);
-    }
-    try {
+
+    try (Connection connection = ConnectionFactory.createConnection(conf);
+      Admin admin = connection.getAdmin()) {
+
+      ReplicationPeerConfig peerConfig = null;
+      try {
+        peerConfig = admin.getReplicationPeerConfig(REGION_REPLICA_REPLICATION_PEER);
+      } catch (ReplicationPeerNotFoundException e) {
+        LOG.warn(

Review comment:
       nit: can be condensed to fewer lines.

##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestClearRegionBlockCache.java
##########
@@ -149,8 +150,8 @@ public void testClearBlockCacheFromAdmin() throws Exception {
 
   @Test
   public void testClearBlockCacheFromAsyncAdmin() throws Exception {
-    AsyncAdmin admin =
-        ConnectionFactory.createAsyncConnection(HTU.getConfiguration()).get().getAdmin();
+    AsyncConnection conn = ConnectionFactory.createAsyncConnection(HTU.getConfiguration()).get();
+    AsyncAdmin admin = conn.getAdmin();

Review comment:
       this should be a try-with-resources too?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org