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 <34427442+ankitjai...@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 <jainan...@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. + } + } + } +}