This is an automated email from the ASF dual-hosted git repository.
tkhurana pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/phoenix.git
The following commit(s) were added to refs/heads/master by this push:
new aad6208399 PHOENIX-7449 Handling already disabled table gracefully
during dropTables execution (#2018)
aad6208399 is described below
commit aad62083996163713674738df1d8f0bc308dfc5c
Author: ritegarg <[email protected]>
AuthorDate: Tue Oct 29 15:29:30 2024 -0700
PHOENIX-7449 Handling already disabled table gracefully during dropTables
execution (#2018)
* PHOENIX-7449 Handling already disabled table gracefully during dropTables
execution
---
.../phoenix/query/ConnectionQueryServicesImpl.java | 22 +++++++++++----
.../query/ConnectionQueryServicesImplTest.java | 33 +++++++++++++++++++++-
2 files changed, 48 insertions(+), 7 deletions(-)
diff --git
a/phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
b/phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
index 2b9b4b55bc..b69b2c06d3 100644
---
a/phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
+++
b/phoenix-core-client/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
@@ -150,6 +150,7 @@ import org.apache.hadoop.hbase.NamespaceNotFoundException;
import org.apache.hadoop.hbase.ServerName;
import org.apache.hadoop.hbase.TableExistsException;
import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.TableNotEnabledException;
import org.apache.hadoop.hbase.client.Admin;
import org.apache.hadoop.hbase.client.Append;
import org.apache.hadoop.hbase.client.CheckAndMutate;
@@ -1790,7 +1791,7 @@ public class ConnectionQueryServicesImpl extends
DelegateQueryServices implement
SQLExceptionCode.INCONSISTENT_NAMESPACE_MAPPING_PROPERTIES.getErrorCode()) {
try {
// In case we wrongly created SYSTEM.CATALOG
or SYSTEM:CATALOG, we should drop it
-
admin.disableTable(TableName.valueOf(physicalTableName));
+ disableTable(admin,
TableName.valueOf(physicalTableName));
admin.deleteTable(TableName.valueOf(physicalTableName));
} catch
(org.apache.hadoop.hbase.TableNotFoundException ignored) {
// Ignore this since it just means that
another client with a similar set of
@@ -1946,7 +1947,7 @@ public class ConnectionQueryServicesImpl extends
DelegateQueryServices implement
TableName tn = TableName.valueOf(tableName);
try (Admin admin = getAdmin()) {
if (!allowOnlineTableSchemaUpdate()) {
- admin.disableTable(tn);
+ disableTable(admin, tn);
admin.modifyTable(newDesc); // TODO: Update to TableDescriptor
admin.enableTable(tn);
} else {
@@ -2239,6 +2240,14 @@ public class ConnectionQueryServicesImpl extends
DelegateQueryServices implement
}
}
+ private void disableTable(Admin admin, TableName tableName) throws
IOException {
+ try {
+ admin.disableTable(tableName);
+ } catch (TableNotEnabledException e) {
+ LOGGER.info("Table already disabled, continuing with next steps",
e);
+ }
+ }
+
private boolean ensureViewIndexTableDropped(byte[] physicalTableName, long
timestamp) throws SQLException {
byte[] physicalIndexName =
MetaDataUtil.getViewIndexPhysicalName(physicalTableName);
boolean wasDeleted = false;
@@ -2250,7 +2259,7 @@ public class ConnectionQueryServicesImpl extends
DelegateQueryServices implement
final ReadOnlyProps props = this.getProps();
final boolean dropMetadata =
props.getBoolean(DROP_METADATA_ATTRIB, DEFAULT_DROP_METADATA);
if (dropMetadata) {
- admin.disableTable(physicalIndexTableName);
+ disableTable(admin, physicalIndexTableName);
admin.deleteTable(physicalIndexTableName);
clearTableRegionCache(physicalIndexTableName);
wasDeleted = true;
@@ -2583,7 +2592,8 @@ public class ConnectionQueryServicesImpl extends
DelegateQueryServices implement
dropTables(Collections.<byte[]>singletonList(tableNameToDelete));
}
- private void dropTables(final List<byte[]> tableNamesToDelete) throws
SQLException {
+ @VisibleForTesting
+ void dropTables(final List<byte[]> tableNamesToDelete) throws SQLException
{
SQLException sqlE = null;
try (Admin admin = getAdmin()) {
if (tableNamesToDelete != null) {
@@ -2591,7 +2601,7 @@ public class ConnectionQueryServicesImpl extends
DelegateQueryServices implement
try {
TableName tn = TableName.valueOf(tableName);
TableDescriptor htableDesc =
this.getTableDescriptor(tableName);
- admin.disableTable(tn);
+ disableTable(admin, tn);
admin.deleteTable(tn);
tableStatsCache.invalidateAll(htableDesc);
clearTableRegionCache(TableName.valueOf(tableName));
@@ -4066,7 +4076,7 @@ public class ConnectionQueryServicesImpl extends
DelegateQueryServices implement
// co-location of data and index regions. If we just
modify the
// table descriptor when online schema change enabled
may reopen
// the region in same region server instead of
following data region.
- admin.disableTable(table.getTableName());
+ disableTable(admin, table.getTableName());
admin.modifyTable(table);
admin.enableTable(table.getTableName());
}
diff --git
a/phoenix-core/src/test/java/org/apache/phoenix/query/ConnectionQueryServicesImplTest.java
b/phoenix-core/src/test/java/org/apache/phoenix/query/ConnectionQueryServicesImplTest.java
index 54a1b29ce7..048bf250aa 100644
---
a/phoenix-core/src/test/java/org/apache/phoenix/query/ConnectionQueryServicesImplTest.java
+++
b/phoenix-core/src/test/java/org/apache/phoenix/query/ConnectionQueryServicesImplTest.java
@@ -36,6 +36,7 @@ import static org.mockito.Mockito.when;
import java.io.IOException;
import java.lang.reflect.Field;
+import java.nio.charset.StandardCharsets;
import java.sql.SQLException;
import java.util.Collections;
import java.util.HashMap;
@@ -45,6 +46,7 @@ import java.util.Map;
import org.apache.hadoop.hbase.HConstants;
import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.client.Admin;
+import org.apache.hadoop.hbase.TableNotEnabledException;
import org.apache.hadoop.hbase.TableNotFoundException;
import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder;
import org.apache.hadoop.hbase.client.Connection;
@@ -61,7 +63,6 @@ import org.apache.phoenix.monitoring.GlobalClientMetrics;
import org.apache.phoenix.util.ReadOnlyProps;
import org.junit.Before;
import org.junit.ClassRule;
-import org.junit.Ignore;
import org.junit.Test;
import org.mockito.Mock;
import org.mockito.Mockito;
@@ -96,6 +97,9 @@ public class ConnectionQueryServicesImplTest {
@Mock
private Table mockTable;
+ @Mock
+ private GuidePostsCacheWrapper mockTableStatsCache;
+
public static final TableDescriptorBuilder SYS_TASK_TDB =
TableDescriptorBuilder
.newBuilder(TableName.valueOf(PhoenixDatabaseMetaData.SYSTEM_TASK_NAME));
public static final TableDescriptorBuilder SYS_TASK_TDB_SP =
TableDescriptorBuilder
@@ -114,6 +118,9 @@ public class ConnectionQueryServicesImplTest {
props =
ConnectionQueryServicesImpl.class.getDeclaredField("connection");
props.setAccessible(true);
props.set(mockCqs, mockConn);
+ props =
ConnectionQueryServicesImpl.class.getDeclaredField("tableStatsCache");
+ props.setAccessible(true);
+ props.set(mockCqs, mockTableStatsCache);
when(mockCqs.checkIfSysMutexExistsAndModifyTTLIfRequired(mockAdmin))
.thenCallRealMethod();
when(mockCqs.updateAndConfirmSplitPolicyForTask(SYS_TASK_TDB))
@@ -124,6 +131,7 @@ public class ConnectionQueryServicesImplTest {
when(mockCqs.getAdmin()).thenCallRealMethod();
when(mockCqs.getTable(Mockito.any())).thenCallRealMethod();
when(mockCqs.getTableIfExists(Mockito.any())).thenCallRealMethod();
+ doCallRealMethod().when(mockCqs).dropTables(Mockito.any());
}
@SuppressWarnings("unchecked")
@@ -343,4 +351,27 @@ public class ConnectionQueryServicesImplTest {
verify(mockConn, Mockito.times(1))
.getTable(TableName.valueOf("SYSTEM:MUTEX"));
}
+
+ @Test
+ public void testDropTablesAlreadyDisabled() throws Exception {
+ when(mockConn.getAdmin()).thenReturn(mockAdmin);
+ doThrow(new
TableNotEnabledException()).when(mockAdmin).disableTable(any());
+ doNothing().when(mockAdmin).deleteTable(any());
+
mockCqs.dropTables(Collections.singletonList("TEST_TABLE".getBytes(StandardCharsets.UTF_8)));
+ verify(mockAdmin,
Mockito.times(1)).disableTable(TableName.valueOf("TEST_TABLE"));
+ verify(mockAdmin,
Mockito.times(1)).deleteTable(TableName.valueOf("TEST_TABLE"));
+ verify(mockConn).getAdmin();
+ }
+
+ @Test
+ public void testDropTablesTableEnabled() throws Exception {
+ when(mockConn.getAdmin()).thenReturn(mockAdmin);
+ doNothing().when(mockAdmin).disableTable(any());
+ doNothing().when(mockAdmin).deleteTable(any());
+ doNothing().when(mockTableStatsCache).invalidateAll();
+
mockCqs.dropTables(Collections.singletonList("TEST_TABLE".getBytes(StandardCharsets.UTF_8)));
+ verify(mockAdmin,
Mockito.times(1)).disableTable(TableName.valueOf("TEST_TABLE"));
+ verify(mockAdmin,
Mockito.times(1)).deleteTable(TableName.valueOf("TEST_TABLE"));
+ verify(mockConn).getAdmin();
+ }
}
