You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@phoenix.apache.org by gj...@apache.org on 2021/10/11 17:27:19 UTC

[phoenix] branch 4.x updated: Throw IOException instead of IllegalArgumentException when RS crashes during index rebuilds. (#1328)

This is an automated email from the ASF dual-hosted git repository.

gjacoby pushed a commit to branch 4.x
in repository https://gitbox.apache.org/repos/asf/phoenix.git


The following commit(s) were added to refs/heads/4.x by this push:
     new 84482e5  Throw IOException instead of IllegalArgumentException when RS crashes during index rebuilds. (#1328)
84482e5 is described below

commit 84482e5692c1b191afa95f4760cc77f8fbf8e0a9
Author: ankitjain64 <34...@users.noreply.github.com>
AuthorDate: Mon Oct 11 10:25:11 2021 -0700

    Throw IOException instead of IllegalArgumentException when RS crashes during index rebuilds. (#1328)
    
    Co-authored-by: Ankit Jain <ja...@salesforce.com>
---
 .../java/org/apache/phoenix/util/ServerUtil.java   | 18 +++++-
 .../org/apache/phoenix/util/ServerUtilTest.java    | 69 ++++++++++++++++++++++
 2 files changed, 84 insertions(+), 3 deletions(-)

diff --git a/phoenix-core/src/main/java/org/apache/phoenix/util/ServerUtil.java b/phoenix-core/src/main/java/org/apache/phoenix/util/ServerUtil.java
index f1075d5..06bb88e 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/util/ServerUtil.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/util/ServerUtil.java
@@ -308,13 +308,13 @@ public class ServerUtil {
             this.connectionType = connectionType;
         }
 
-        private ClusterConnection getConnection() throws IOException {
+        public ClusterConnection getConnection() throws IOException {
             return ConnectionFactory.getConnection(connectionType, conf, server);
         }
 
         @Override
         public HTableInterface getTable(ImmutableBytesPtr tablename) throws IOException {
-            return getConnection().getTable(tablename.copyBytesIfNecessary());
+            return getTable(tablename, null);
         }
 
         @Override
@@ -328,7 +328,19 @@ public class ServerUtil {
         @Override
         public HTableInterface getTable(ImmutableBytesPtr tablename, ExecutorService pool)
                 throws IOException {
-            return getConnection().getTable(tablename.copyBytesIfNecessary(), pool);
+            ClusterConnection connection = null;
+            try {
+                connection = getConnection();
+                if (pool == null) {
+                    return connection.getTable(tablename.copyBytesIfNecessary());
+                }
+                return connection.getTable(tablename.copyBytesIfNecessary(), pool);
+            } catch (IllegalArgumentException e) {
+                if (connection == null || connection.isClosed()) {
+                    throw new IOException("Connection is null or closed. Please retry again.");
+                }
+                throw e;
+            }
         }
     }
 
diff --git a/phoenix-core/src/test/java/org/apache/phoenix/util/ServerUtilTest.java b/phoenix-core/src/test/java/org/apache/phoenix/util/ServerUtilTest.java
new file mode 100644
index 0000000..5da19ff
--- /dev/null
+++ b/phoenix-core/src/test/java/org/apache/phoenix/util/ServerUtilTest.java
@@ -0,0 +1,69 @@
+/**
+ *  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.phoenix.util;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.DoNotRetryIOException;
+import org.apache.hadoop.hbase.client.ClusterConnection;
+import org.apache.hadoop.hbase.regionserver.HRegionServer;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.phoenix.hbase.index.util.ImmutableBytesPtr;
+import org.apache.phoenix.query.HBaseFactoryProvider;
+import org.junit.Assert;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+import java.io.IOException;
+import java.util.concurrent.ExecutorService;
+
+public class ServerUtilTest {
+
+    @Test
+    public void testCoprocessorHConnectionGetTableWithClosedConnection() throws Exception {
+        Configuration conf = HBaseFactoryProvider.getConfigurationFactory().getConfiguration();
+        HRegionServer server = Mockito.mock(HRegionServer.class);
+        ServerUtil.ConnectionType connectionType = ServerUtil.ConnectionType.INDEX_WRITER_CONNECTION;
+
+        // Mock ClusterConnection object to throw IllegalArgumentException.
+        ClusterConnection connection = Mockito.mock(ClusterConnection.class);
+        Mockito.doThrow(new IllegalArgumentException()).when(connection).getTable(
+            Mockito.<byte[]>any());
+        Mockito.doThrow(new IllegalArgumentException()).when(connection).getTable(
+            Mockito.<byte[]>any(), Mockito.<ExecutorService>any());
+        Mockito.doReturn(true).when(connection).isClosed();
+
+        // Spy CoprocessorHConnectionTableFactory
+        ServerUtil.CoprocessorHConnectionTableFactory coprocHTableFactory =  new ServerUtil.
+            CoprocessorHConnectionTableFactory(conf, server, connectionType);
+        ServerUtil.CoprocessorHConnectionTableFactory spyedObj = Mockito.spy(coprocHTableFactory);
+        Mockito.doReturn(connection).when(spyedObj).getConnection();
+
+        try {
+            spyedObj.getTable(new ImmutableBytesPtr(Bytes.toBytes("test_table")));
+            Assert.fail("IOException exception expected as connection was closed");
+        }catch(DoNotRetryIOException e) {
+            Assert.fail("DoNotRetryIOException not expected instead should throw IOException");
+        }catch (IOException e1) {
+            try {
+                spyedObj.getTable(new ImmutableBytesPtr(Bytes.toBytes("test_table")), null);
+                Assert.fail("IOException exception expected as connection was closed");
+            } catch (IOException e2) {
+                // IO Exception is expected. Should fail is any other exception.
+            }
+        }
+    }
+}