PHOENIX-5621 - IndexUpgradeTool uses wrong priority and unnecessary properties for GlobalIndexChecker
diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/ParameterizedIndexUpgradeToolIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/ParameterizedIndexUpgradeToolIT.java
index 1df46a8..16f99e3 100644
--- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/ParameterizedIndexUpgradeToolIT.java
+++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/ParameterizedIndexUpgradeToolIT.java
@@ -18,9 +18,11 @@
package org.apache.phoenix.end2end;
import com.google.common.collect.Maps;
+import org.apache.hadoop.hbase.HTableDescriptor;
import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.client.Admin;
import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.phoenix.end2end.index.IndexCoprocIT;
import org.apache.phoenix.hbase.index.IndexRegionObserver;
import org.apache.phoenix.hbase.index.Indexer;
import org.apache.phoenix.index.GlobalIndexChecker;
@@ -230,18 +232,22 @@
throws IOException {
if (mutable) {
for (String table : tableList) {
+ HTableDescriptor indexDesc = admin.getTableDescriptor(TableName.valueOf(table));
Assert.assertTrue("Can't find IndexRegionObserver for " + table,
- admin.getTableDescriptor(TableName.valueOf(table))
- .hasCoprocessor(IndexRegionObserver.class.getName()));
+ indexDesc.hasCoprocessor(IndexRegionObserver.class.getName()));
Assert.assertFalse("Found Indexer on " + table,
- admin.getTableDescriptor(TableName.valueOf(table))
- .hasCoprocessor(Indexer.class.getName()));
+ indexDesc.hasCoprocessor(Indexer.class.getName()));
+ IndexCoprocIT.assertCoprocConfig(indexDesc, IndexRegionObserver.class.getName(),
+ IndexCoprocIT.INDEX_REGION_OBSERVER_CONFIG);
}
+
}
for (String index : indexList) {
+ HTableDescriptor indexDesc = admin.getTableDescriptor(TableName.valueOf(index));
Assert.assertTrue("Couldn't find GlobalIndexChecker on " + index,
- admin.getTableDescriptor(TableName.valueOf(index))
- .hasCoprocessor(GlobalIndexChecker.class.getName()));
+ indexDesc.hasCoprocessor(GlobalIndexChecker.class.getName()));
+ IndexCoprocIT.assertCoprocConfig(indexDesc, GlobalIndexChecker.class.getName(),
+ IndexCoprocIT.GLOBAL_INDEX_CHECKER_CONFIG);
}
// Transactional indexes should not have new coprocessors
for (String index : TRANSACTIONAL_INDEXES_LIST) {
@@ -260,12 +266,13 @@
throws IOException {
if (mutable) {
for (String table : tableList) {
+ HTableDescriptor indexDesc = admin.getTableDescriptor(TableName.valueOf(table));
Assert.assertTrue("Can't find Indexer for " + table,
- admin.getTableDescriptor(TableName.valueOf(table))
- .hasCoprocessor(Indexer.class.getName()));
+ indexDesc.hasCoprocessor(Indexer.class.getName()));
Assert.assertFalse("Found IndexRegionObserver on " + table,
- admin.getTableDescriptor(TableName.valueOf(table))
- .hasCoprocessor(IndexRegionObserver.class.getName()));
+ indexDesc.hasCoprocessor(IndexRegionObserver.class.getName()));
+ IndexCoprocIT.assertCoprocConfig(indexDesc, Indexer.class.getName(),
+ IndexCoprocIT.INDEXER_CONFIG);
}
}
for (String index : indexList) {
diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/index/IndexCoprocIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/index/IndexCoprocIT.java
index cdf45e2..839d46b 100644
--- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/index/IndexCoprocIT.java
+++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/index/IndexCoprocIT.java
@@ -22,16 +22,18 @@
import org.apache.hadoop.hbase.HTableDescriptor;
import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.client.Admin;
-import org.apache.hadoop.hbase.client.HTable;
+import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
import org.apache.hadoop.hbase.util.Bytes;
import org.apache.phoenix.end2end.ParallelStatsDisabledIT;
import org.apache.phoenix.hbase.index.IndexRegionObserver;
import org.apache.phoenix.hbase.index.Indexer;
import org.apache.phoenix.hbase.index.covered.NonTxIndexBuilder;
import org.apache.phoenix.index.GlobalIndexChecker;
+import org.apache.phoenix.index.PhoenixIndexBuilder;
import org.apache.phoenix.index.PhoenixIndexCodec;
import org.apache.phoenix.jdbc.PhoenixConnection;
import org.apache.phoenix.query.QueryServices;
+import org.apache.phoenix.query.QueryServicesOptions;
import org.apache.phoenix.util.SchemaUtil;
import org.junit.Assert;
import org.junit.Test;
@@ -51,6 +53,18 @@
public class IndexCoprocIT extends ParallelStatsDisabledIT {
private boolean isNamespaceMapped = false;
private boolean isMultiTenant = false;
+ public static final String GLOBAL_INDEX_CHECKER_CONFIG =
+ "|org.apache.phoenix.index.GlobalIndexChecker|805306365|";
+ public static final String INDEX_REGION_OBSERVER_CONFIG =
+ "|org.apache.phoenix.hbase.index.IndexRegionObserver" +
+ "|805306366|org.apache.hadoop.hbase.index.codec.class=" +
+ "org.apache.phoenix.index.PhoenixIndexCodec," +
+ "index.builder=org.apache.phoenix.index.PhoenixIndexBuilder";
+ public static final String INDEXER_CONFIG =
+ "|org.apache.phoenix.hbase.index.Indexer" +
+ "|805306366|org.apache.hadoop.hbase.index.codec.class=" +
+ "org.apache.phoenix.index.PhoenixIndexCodec," +
+ "index.builder=org.apache.phoenix.index.PhoenixIndexBuilder";
public IndexCoprocIT(boolean isMultiTenant){
this.isMultiTenant = isMultiTenant;
@@ -86,8 +100,8 @@
Map<String, String> props = new HashMap<String, String>();
props.put(NonTxIndexBuilder.CODEC_CLASS_NAME_KEY, PhoenixIndexCodec.class.getName());
- Indexer.enableIndexing(baseDescriptor, NonTxIndexBuilder.class,
- props, 100);
+ Indexer.enableIndexing(baseDescriptor, PhoenixIndexBuilder.class,
+ props, QueryServicesOptions.DEFAULT_COPROCESSOR_PRIORITY);
admin.modifyTable(baseDescriptor.getTableName(), baseDescriptor);
baseDescriptor = admin.getTableDescriptor(TableName.valueOf(physicalTableName));
indexDescriptor = admin.getTableDescriptor(TableName.valueOf(physicalIndexName));
@@ -153,6 +167,8 @@
private void assertUsingOldCoprocs(HTableDescriptor baseDescriptor,
HTableDescriptor indexDescriptor) {
assertCoprocsContains(Indexer.class, baseDescriptor);
+ assertCoprocConfig(baseDescriptor, Indexer.class.getName(),
+ INDEXER_CONFIG);
assertCoprocsNotContains(IndexRegionObserver.class, baseDescriptor);
assertCoprocsNotContains(IndexRegionObserver.class, indexDescriptor);
assertCoprocsNotContains(GlobalIndexChecker.class, indexDescriptor);
@@ -166,9 +182,13 @@
private void assertUsingNewCoprocs(HTableDescriptor baseDescriptor,
HTableDescriptor indexDescriptor) {
assertCoprocsContains(IndexRegionObserver.class, baseDescriptor);
+ assertCoprocConfig(baseDescriptor, IndexRegionObserver.class.getName(),
+ INDEX_REGION_OBSERVER_CONFIG);
assertCoprocsNotContains(Indexer.class, baseDescriptor);
assertCoprocsNotContains(Indexer.class, indexDescriptor);
assertCoprocsContains(GlobalIndexChecker.class, indexDescriptor);
+ assertCoprocConfig(indexDescriptor, GlobalIndexChecker.class.getName(),
+ GLOBAL_INDEX_CHECKER_CONFIG);
}
private void assertCoprocsContains(Class clazz, HTableDescriptor descriptor) {
@@ -196,6 +216,28 @@
return foundCoproc;
}
+ public static void assertCoprocConfig(HTableDescriptor indexDesc,
+ String className, String expectedConfigValue){
+ boolean foundConfig = false;
+ for (Map.Entry<ImmutableBytesWritable, ImmutableBytesWritable> entry :
+ indexDesc.getValues().entrySet()){
+ String propKey = Bytes.toString(entry.getKey().get());
+ String propValue = Bytes.toString(entry.getValue().get());
+ //Unfortunately, a good API to read coproc properties didn't show up until
+ //HBase 2.0. Doing this the painful String-matching way to be compatible with 1.x
+ if (propKey.contains("coprocessor")){
+ if (propValue.contains(className)){
+ Assert.assertEquals(className + " is configured incorrectly",
+ expectedConfigValue,
+ propValue);
+ foundConfig = true;
+ break;
+ }
+ }
+ }
+ Assert.assertTrue("Couldn't find config for " + className, foundConfig);
+ }
+
private void removeCoproc(Class clazz, HTableDescriptor descriptor, Admin admin) throws Exception {
descriptor.removeCoprocessor(clazz.getName());
admin.modifyTable(descriptor.getTableName(), descriptor);
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/IndexUpgradeTool.java b/phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/IndexUpgradeTool.java
index 437738a..ef8c492 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/IndexUpgradeTool.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/IndexUpgradeTool.java
@@ -19,7 +19,6 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Strings;
-import com.google.gson.Gson;
import org.apache.commons.cli.CommandLine;
import org.apache.commons.cli.CommandLineParser;
import org.apache.commons.cli.HelpFormatter;
@@ -108,6 +107,7 @@
private HashMap<String, HashSet<String>> tablesAndIndexes = new HashMap<>();
private HashMap<String, String> prop = new HashMap<>();
+ private HashMap<String, String> emptyProp = new HashMap<>();
private boolean dryRun, upgrade, syncRebuild;
private String operation;
@@ -461,11 +461,17 @@
}
private void addCoprocessor(Admin admin, String tableName, HTableDescriptor tableDesc,
- String coprocName) throws IOException {
+ String coprocName) throws IOException {
+ addCoprocessor(admin, tableName, tableDesc, coprocName,
+ QueryServicesOptions.DEFAULT_COPROCESSOR_PRIORITY, prop);
+ }
+
+ private void addCoprocessor(Admin admin, String tableName, HTableDescriptor tableDesc,
+ String coprocName, int priority, Map<String, String> propsToAdd) throws IOException {
if (!admin.getTableDescriptor(TableName.valueOf(tableName)).hasCoprocessor(coprocName)) {
if (!dryRun) {
tableDesc.addCoprocessor(coprocName,
- null, QueryServicesOptions.DEFAULT_COPROCESSOR_PRIORITY, prop);
+ null, priority, propsToAdd);
}
LOGGER.info("Loaded " + coprocName + " coprocessor on table " + tableName);
} else {
@@ -490,7 +496,10 @@
for (String indexName : indexes) {
HTableDescriptor indexTableDesc = admin.getTableDescriptor(TableName.valueOf(indexName));
if (upgrade) {
- addCoprocessor(admin, indexName, indexTableDesc, GlobalIndexChecker.class.getName());
+ //GlobalIndexChecker needs to be a "lower" priority than all the others so that it
+ //goes first. It also doesn't get the codec props the IndexRegionObserver needs
+ addCoprocessor(admin, indexName, indexTableDesc, GlobalIndexChecker.class.getName(),
+ QueryServicesOptions.DEFAULT_COPROCESSOR_PRIORITY -1, emptyProp);
} else {
removeCoprocessor(admin, indexName, indexTableDesc,
GlobalIndexChecker.class.getName());
