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 18:01:28 UTC
[phoenix] branch 5.1 updated: PHOENIX-6548: Throw IOException
instead of IllegalArgumentException when RS crashes during index rebuilds
(#1329)
This is an automated email from the ASF dual-hosted git repository.
gjacoby pushed a commit to branch 5.1
in repository https://gitbox.apache.org/repos/asf/phoenix.git
The following commit(s) were added to refs/heads/5.1 by this push:
new bba91da PHOENIX-6548: Throw IOException instead of IllegalArgumentException when RS crashes during index rebuilds (#1329)
bba91da is described below
commit bba91dae16d80cc02029b0af33631080e51689f5
Author: ankitjain64 <34...@users.noreply.github.com>
AuthorDate: Mon Oct 11 10:27:06 2021 -0700
PHOENIX-6548: Throw IOException instead of IllegalArgumentException when RS crashes during index rebuilds (#1329)
Co-authored-by: Ankit Jain <ja...@salesforce.com>
---
.../hbase/index/write/IndexWriterUtils.java | 20 +++++--
.../org/apache/phoenix/util/ServerUtilTest.java | 68 ++++++++++++++++++++++
2 files changed, 84 insertions(+), 4 deletions(-)
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/hbase/index/write/IndexWriterUtils.java b/phoenix-core/src/main/java/org/apache/phoenix/hbase/index/write/IndexWriterUtils.java
index 4dbe579..01f587b 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/hbase/index/write/IndexWriterUtils.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/hbase/index/write/IndexWriterUtils.java
@@ -106,7 +106,7 @@ public class IndexWriterUtils {
* factory was added as a workaround to the bug reported in
* https://issues.apache.org/jira/browse/HBASE-18359
*/
- private static class CoprocessorHConnectionTableFactory implements HTableFactory {
+ public static class CoprocessorHConnectionTableFactory implements HTableFactory {
@GuardedBy("CoprocessorHConnectionTableFactory.this")
private RegionCoprocessorEnvironment env;
private ConnectionType connectionType;
@@ -116,12 +116,12 @@ public class IndexWriterUtils {
this.connectionType = connectionType;
}
- private Connection getConnection() throws IOException {
+ public Connection getConnection() throws IOException {
return ConnectionFactory.getConnection(connectionType, env);
}
@Override
public Table getTable(ImmutableBytesPtr tablename) throws IOException {
- return getConnection().getTable(TableName.valueOf(tablename.copyBytesIfNecessary()));
+ return getTable(tablename, null);
}
@Override
@@ -132,7 +132,19 @@ public class IndexWriterUtils {
@Override
public Table getTable(ImmutableBytesPtr tablename, ExecutorService pool)
throws IOException {
- return getConnection().getTable(TableName.valueOf(tablename.copyBytesIfNecessary()), pool);
+ Connection connection = null;
+ try {
+ connection = getConnection();
+ if (pool == null) {
+ return connection.getTable(TableName.valueOf(tablename.copyBytesIfNecessary()));
+ }
+ return connection.getTable(TableName.valueOf(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..997f94a
--- /dev/null
+++ b/phoenix-core/src/test/java/org/apache/phoenix/util/ServerUtilTest.java
@@ -0,0 +1,68 @@
+/**
+ * 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.client.Connection;
+import org.apache.hadoop.hbase.DoNotRetryIOException;
+import org.apache.hadoop.hbase.coprocessor.RegionCoprocessorEnvironment;
+import org.apache.hadoop.hbase.regionserver.HRegionServer;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.phoenix.hbase.index.table.HTableFactory;
+import org.apache.phoenix.hbase.index.util.ImmutableBytesPtr;
+import org.apache.phoenix.hbase.index.write.IndexWriterUtils;
+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 {
+ // Mock Connection object to throw IllegalArgumentException.
+ Connection connection = Mockito.mock(Connection.class);
+ Mockito.doThrow(new IllegalArgumentException()).when(connection).getTable(Mockito.any());
+ Mockito.doThrow(new IllegalArgumentException()).when(connection).getTable(
+ Mockito.any(), Mockito.<ExecutorService>any());
+ Mockito.doReturn(true).when(connection).isClosed();
+
+ // Spy CoprocessorHConnectionTableFactory
+ RegionCoprocessorEnvironment mockEnv = Mockito.mock(RegionCoprocessorEnvironment.class);
+ HTableFactory hTableFactory = IndexWriterUtils.getDefaultDelegateHTableFactory(mockEnv);
+ IndexWriterUtils.CoprocessorHConnectionTableFactory spyedObj = (IndexWriterUtils.
+ CoprocessorHConnectionTableFactory)Mockito.spy(hTableFactory);
+ 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.
+ }
+ }
+ }
+}