This is an automated email from the ASF dual-hosted git repository.
stoty 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 db16b53b9d PHOENIX-6953 Creating indexes on a table with old indexing
leads to inconsistent co-processors
db16b53b9d is described below
commit db16b53b9d0d7f7c142799f417fb90e1d3d8fe75
Author: Istvan Toth <st...@apache.org>
AuthorDate: Thu May 11 15:58:26 2023 +0200
PHOENIX-6953 Creating indexes on a table with old indexing leads to
inconsistent co-processors
---
.../org/apache/phoenix/end2end/CreateTableIT.java | 101 ++++++++++++++++++++-
.../phoenix/query/ConnectionQueryServicesImpl.java | 32 ++++---
2 files changed, 119 insertions(+), 14 deletions(-)
diff --git
a/phoenix-core/src/it/java/org/apache/phoenix/end2end/CreateTableIT.java
b/phoenix-core/src/it/java/org/apache/phoenix/end2end/CreateTableIT.java
index 680dba1baf..8e622f40b9 100644
--- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/CreateTableIT.java
+++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/CreateTableIT.java
@@ -17,6 +17,7 @@
*/
package org.apache.phoenix.end2end;
+import static org.apache.phoenix.mapreduce.index.IndexUpgradeTool.ROLLBACK_OP;
import static org.apache.phoenix.util.TestUtil.TEST_PROPERTIES;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
@@ -37,20 +38,22 @@ import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Properties;
+import java.util.UUID;
import org.apache.hadoop.hbase.HConstants;
import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.client.Admin;
import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder;
-import org.apache.hadoop.hbase.ipc.PhoenixRpcSchedulerFactory;
-import org.apache.hadoop.hbase.regionserver.BloomType;
import org.apache.hadoop.hbase.client.TableDescriptor;
import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
+import org.apache.hadoop.hbase.ipc.PhoenixRpcSchedulerFactory;
+import org.apache.hadoop.hbase.regionserver.BloomType;
import org.apache.phoenix.exception.SQLExceptionCode;
import org.apache.phoenix.jdbc.PhoenixConnection;
import org.apache.phoenix.jdbc.PhoenixDatabaseMetaData;
import org.apache.phoenix.jdbc.PhoenixStatement;
+import org.apache.phoenix.mapreduce.index.IndexUpgradeTool;
import org.apache.phoenix.query.BaseTest;
import org.apache.phoenix.query.KeyRange;
import org.apache.phoenix.query.QueryConstants;
@@ -65,7 +68,6 @@ import org.apache.phoenix.schema.PTableType;
import org.apache.phoenix.schema.SchemaNotFoundException;
import org.apache.phoenix.schema.TableAlreadyExistsException;
import org.apache.phoenix.schema.TableNotFoundException;
-import org.apache.phoenix.schema.export.DefaultSchemaRegistryRepository;
import org.apache.phoenix.util.EnvironmentEdgeManager;
import org.apache.phoenix.util.PhoenixRuntime;
import org.apache.phoenix.util.PropertiesUtil;
@@ -1574,6 +1576,99 @@ public class CreateTableIT extends
ParallelStatsDisabledIT {
.getColumnQualifierBytes()));
}
+ // Test for PHOENIX-6953
+ @Test
+ public void testCoprocessorsForCreateIndexOnOldImplementation() throws
Exception {
+ String tableName = generateUniqueName();
+ String index1Name = generateUniqueName();
+ String index2Name = generateUniqueName();
+
+ String ddl =
+ "create table " + tableName + " ( k integer PRIMARY KEY," + "
v1 integer,"
+ + " v2 integer)";
+ String index1Ddl = "create index " + index1Name + " on " + tableName
+ " (v1)";
+ String index2Ddl = "create index " + index2Name + " on " + tableName
+ " (v2)";
+
+ Properties props = new Properties();
+ Admin admin = driver.getConnectionQueryServices(getUrl(),
props).getAdmin();
+
+ try (Connection conn = DriverManager.getConnection(getUrl());
+ Statement stmt = conn.createStatement();) {
+ stmt.execute(ddl);
+ stmt.execute(index1Ddl);
+
+ TableDescriptor index1DescriptorBefore =
+ admin.getDescriptor(TableName.valueOf(index1Name));
+ assertTrue(index1DescriptorBefore
+
.hasCoprocessor(org.apache.phoenix.index.GlobalIndexChecker.class.getName()));
+
+ // Now roll back to the old indexing
+ IndexUpgradeTool iut =
+ new IndexUpgradeTool(ROLLBACK_OP, tableName, null,
+ "/tmp/index_upgrade_" +
UUID.randomUUID().toString(), false, null,
+ false);
+ iut.setConf(getUtility().getConfiguration());
+ iut.prepareToolSetup();
+ assertEquals(0, iut.executeTool());
+
+ TableDescriptor index1DescriptorAfter =
+ admin.getDescriptor(TableName.valueOf(index1Name));
+ assertFalse(index1DescriptorAfter
+
.hasCoprocessor(org.apache.phoenix.index.GlobalIndexChecker.class.getName()));
+
+ // New index must not have GlobalIndexChecker
+ stmt.execute(index2Ddl);
+ TableDescriptor index2Descriptor =
admin.getDescriptor(TableName.valueOf(index2Name));
+ assertFalse(index2Descriptor
+
.hasCoprocessor(org.apache.phoenix.index.GlobalIndexChecker.class.getName()));
+ }
+ }
+
+ // Test for PHOENIX-6953
+ @Test
+ public void
testCoprocessorsForTransactionalCreateIndexOnOldImplementation() throws
Exception {
+ String tableName = generateUniqueName();
+ String index1Name = generateUniqueName();
+
+ String ddl =
+ "create table " + tableName + " ( k integer PRIMARY KEY," + "
v1 integer,"
+ + " v2 integer) TRANSACTIONAL=TRUE";
+ String index1Ddl = "create index " + index1Name + " on " + tableName
+ " (v1)";
+
+ Properties props = new Properties();
+ Admin admin = driver.getConnectionQueryServices(getUrl(),
props).getAdmin();
+
+ try (Connection conn = DriverManager.getConnection(getUrl());
+ Statement stmt = conn.createStatement();) {
+ stmt.execute(ddl);
+ stmt.execute(index1Ddl);
+
+ // Transactional indexes don't have GlobalIndexChecker
+ TableDescriptor index1DescriptorBefore =
+ admin.getDescriptor(TableName.valueOf(index1Name));
+ assertFalse(index1DescriptorBefore
+
.hasCoprocessor(org.apache.phoenix.index.GlobalIndexChecker.class.getName()));
+
+ // Now roll back to the old indexing
+ IndexUpgradeTool iut =
+ new IndexUpgradeTool(ROLLBACK_OP, tableName, null,
+ "/tmp/index_upgrade_" +
UUID.randomUUID().toString(), false, null,
+ false);
+ iut.setConf(getUtility().getConfiguration());
+ iut.prepareToolSetup();
+ assertEquals(0, iut.executeTool());
+
+ // Make sure we don't add GlobalIndexChecker
+ TableDescriptor index1DescriptorAfter =
+ admin.getDescriptor(TableName.valueOf(index1Name));
+ assertFalse(index1DescriptorAfter
+
.hasCoprocessor(org.apache.phoenix.index.GlobalIndexChecker.class.getName()));
+
+ // We should also test for setting / unsetting the transactional
status, but both are
+ // forbidden at the time of writing.
+ }
+ }
+
public static long verifyLastDDLTimestamp(String tableFullName, long
startTS, Connection conn) throws SQLException {
long endTS = EnvironmentEdgeManager.currentTimeMillis();
//Now try the PTable API
diff --git
a/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
b/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
index cb119a8401..84c742fd13 100644
---
a/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
+++
b/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
@@ -939,6 +939,7 @@ public class ConnectionQueryServicesImpl extends
DelegateQueryServices implement
:
TableDescriptorBuilder.newBuilder(TableName.valueOf(physicalTableName));
ColumnFamilyDescriptor dataTableColDescForIndexTablePropSyncing = null;
+ boolean doNotAddGlobalIndexChecker = false;
if (tableType == PTableType.INDEX ||
MetaDataUtil.isViewIndex(Bytes.toString(physicalTableName))) {
byte[] defaultFamilyBytes =
defaultFamilyName == null ?
QueryConstants.DEFAULT_COLUMN_FAMILY_BYTES : Bytes.toBytes(defaultFamilyName);
@@ -964,6 +965,10 @@ public class ConnectionQueryServicesImpl extends
DelegateQueryServices implement
if (dataTableColDescForIndexTablePropSyncing == null) {
dataTableColDescForIndexTablePropSyncing =
baseTableDesc.getColumnFamilies()[0];
}
+ if
(baseTableDesc.hasCoprocessor(org.apache.phoenix.hbase.index.Indexer.class.getName()))
{
+ // The base table still uses the old indexing
+ doNotAddGlobalIndexChecker = true;
+ }
}
// By default, do not automatically rebuild/catch up an index on a
write failure
// Add table-specific properties to the table descriptor
@@ -1012,7 +1017,7 @@ public class ConnectionQueryServicesImpl extends
DelegateQueryServices implement
}
}
addCoprocessors(physicalTableName, tableDescriptorBuilder,
- tableType, tableProps, existingDesc);
+ tableType, tableProps, existingDesc, doNotAddGlobalIndexChecker);
// PHOENIX-3072: Set index priority if this is a system table or index
table
if (tableType == PTableType.SYSTEM) {
@@ -1039,8 +1044,8 @@ public class ConnectionQueryServicesImpl extends
DelegateQueryServices implement
private void addCoprocessors(byte[] tableName, TableDescriptorBuilder
builder,
- PTableType tableType, Map<String,Object>
tableProps,
- TableDescriptor existingDesc) throws
SQLException {
+ PTableType tableType, Map<String, Object> tableProps,
TableDescriptor existingDesc,
+ boolean doNotAddGlobalIndexChecker) throws SQLException {
// The phoenix jar must be available on HBase classpath
int priority = props.getInt(QueryServices.COPROCESSOR_PRIORITY_ATTRIB,
QueryServicesOptions.DEFAULT_COPROCESSOR_PRIORITY);
try {
@@ -1071,8 +1076,11 @@ public class ConnectionQueryServicesImpl extends
DelegateQueryServices implement
if
(newDesc.hasCoprocessor(IndexRegionObserver.class.getName())) {
builder.removeCoprocessor(IndexRegionObserver.class.getName());
}
-
builder.setCoprocessor(CoprocessorDescriptorBuilder.newBuilder(GlobalIndexChecker.class.getName())
- .setPriority(priority - 1).build());
+ if (!doNotAddGlobalIndexChecker) {
+ builder.setCoprocessor(CoprocessorDescriptorBuilder
+ .newBuilder(GlobalIndexChecker.class.getName())
+ .setPriority(priority - 1).build());
+ }
}
}
@@ -2609,7 +2617,9 @@ public class ConnectionQueryServicesImpl extends
DelegateQueryServices implement
Map<TableDescriptor, TableDescriptor> oldToNewTableDescriptors)
throws SQLException {
byte[] physicalTableName = table.getPhysicalName().getBytes();
try (Admin admin = getAdmin()) {
- setTransactional(physicalTableName, tableDescriptorBuilder,
table.getType(), txValue, tableProps);
+ TableDescriptor baseDesc =
admin.getDescriptor(TableName.valueOf(physicalTableName));
+ boolean hasOldIndexing =
baseDesc.hasCoprocessor(org.apache.phoenix.hbase.index.Indexer.class.getName());
+ setTransactional(physicalTableName, tableDescriptorBuilder,
table.getType(), txValue, tableProps, hasOldIndexing);
Map<String, Object> indexTableProps;
if (txValue == null) {
indexTableProps = Collections.emptyMap();
@@ -2655,7 +2665,7 @@ public class ConnectionQueryServicesImpl extends
DelegateQueryServices implement
indexDescriptorBuilder.setColumnFamily(indexColDescriptor.build());
}
}
- setTransactional(index.getPhysicalName().getBytes(),
indexDescriptorBuilder, index.getType(), txValue, indexTableProps);
+ setTransactional(index.getPhysicalName().getBytes(),
indexDescriptorBuilder, index.getType(), txValue, indexTableProps,
hasOldIndexing);
descriptorsToUpdate.add(indexDescriptorBuilder.build());
}
try {
@@ -2669,7 +2679,7 @@ public class ConnectionQueryServicesImpl extends
DelegateQueryServices implement
}
TableDescriptorBuilder indexDescriptorBuilder =
TableDescriptorBuilder.newBuilder(intermedIndexDesc);
setSharedIndexMaxVersion(table,
tableDescriptorBuilder.build(), indexDescriptorBuilder);
-
setTransactional(MetaDataUtil.getViewIndexPhysicalName(physicalTableName),
indexDescriptorBuilder, PTableType.INDEX, txValue, indexTableProps);
+
setTransactional(MetaDataUtil.getViewIndexPhysicalName(physicalTableName),
indexDescriptorBuilder, PTableType.INDEX, txValue, indexTableProps,
hasOldIndexing);
descriptorsToUpdate.add(indexDescriptorBuilder.build());
} catch (org.apache.hadoop.hbase.TableNotFoundException ignore) {
// Ignore, as we may never have created a view index table
@@ -2685,7 +2695,7 @@ public class ConnectionQueryServicesImpl extends
DelegateQueryServices implement
}
TableDescriptorBuilder indexDescriptorBuilder =
TableDescriptorBuilder.newBuilder(intermedIndexDesc);
setSharedIndexMaxVersion(table,
tableDescriptorBuilder.build(), indexDescriptorBuilder);
-
setTransactional(MetaDataUtil.getViewIndexPhysicalName(physicalTableName),
indexDescriptorBuilder, PTableType.INDEX, txValue, indexTableProps);
+
setTransactional(MetaDataUtil.getViewIndexPhysicalName(physicalTableName),
indexDescriptorBuilder, PTableType.INDEX, txValue, indexTableProps,
hasOldIndexing);
descriptorsToUpdate.add(indexDescriptorBuilder.build());
} catch (org.apache.hadoop.hbase.TableNotFoundException ignore) {
// Ignore, as we may never have created a local index
@@ -2741,13 +2751,13 @@ public class ConnectionQueryServicesImpl extends
DelegateQueryServices implement
}
}
}
- private void setTransactional(byte[] physicalTableName,
TableDescriptorBuilder tableDescriptorBuilder, PTableType tableType, String
txValue, Map<String, Object> tableProps) throws SQLException {
+ private void setTransactional(byte[] physicalTableName,
TableDescriptorBuilder tableDescriptorBuilder, PTableType tableType, String
txValue, Map<String, Object> tableProps, boolean hasOldIndexing) throws
SQLException {
if (txValue == null) {
tableDescriptorBuilder.removeValue(Bytes.toBytes(PhoenixTransactionContext.READ_NON_TX_DATA));
} else {
tableDescriptorBuilder.setValue(PhoenixTransactionContext.READ_NON_TX_DATA,
txValue);
}
- this.addCoprocessors(physicalTableName, tableDescriptorBuilder,
tableType, tableProps, null);
+ this.addCoprocessors(physicalTableName, tableDescriptorBuilder,
tableType, tableProps, null, hasOldIndexing);
}
private Map<TableDescriptor, TableDescriptor>
separateAndValidateProperties(PTable table,
