commit fe7befcd1fbd89944b2618e1c190bb25fdf6adca Author: Alan Gates Date: Wed Oct 21 11:45:24 2015 -0700 HIVE-12170 normalize HBase metastore connection configuration diff --git itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/hbase/HBaseIntegrationTests.java itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/hbase/HBaseIntegrationTests.java index 5b82579..3f5da4a 100644 --- itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/hbase/HBaseIntegrationTests.java +++ itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/hbase/HBaseIntegrationTests.java @@ -18,16 +18,11 @@ */ package org.apache.hadoop.hive.metastore.hbase; -import co.cask.tephra.hbase10.coprocessor.TransactionProcessor; - import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.HBaseConfiguration; import org.apache.hadoop.hbase.HBaseTestingUtility; -import org.apache.hadoop.hbase.HColumnDescriptor; -import org.apache.hadoop.hbase.HTableDescriptor; -import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.client.HBaseAdmin; import org.apache.hadoop.hive.cli.CliSessionState; import org.apache.hadoop.hive.conf.HiveConf; @@ -38,7 +33,6 @@ import java.io.IOException; import java.util.HashMap; -import java.util.List; import java.util.Map; /** @@ -85,7 +79,7 @@ protected void setupDriver() { // This chicanery is necessary to make the driver work. Hive tests need the pfile file // system, while the hbase one uses something else. So first make sure we've configured our // hbase connection, then get a new config file and populate it as desired. - HBaseReadWrite.getInstance(conf); + HBaseReadWrite.setConf(conf); conf = new HiveConf(); conf.setVar(HiveConf.ConfVars.DYNAMICPARTITIONINGMODE, "nonstrict"); conf.setVar(HiveConf.ConfVars.METASTORE_RAW_STORE_IMPL, diff --git itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/hbase/TestHBaseStoreIntegration.java itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/hbase/TestHBaseStoreIntegration.java index 8b0b431..8750a05 100644 --- itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/hbase/TestHBaseStoreIntegration.java +++ itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/hbase/TestHBaseStoreIntegration.java @@ -837,7 +837,8 @@ public void userToRoleMap() throws Exception { store.grantRole(role2, user1, PrincipalType.USER, "admin", PrincipalType.ROLE, false); store.grantRole(role1, user2, PrincipalType.USER, "bob", PrincipalType.USER, false); - roles = HBaseReadWrite.getInstance(conf).getUserRoles(user2); + HBaseReadWrite.setConf(conf); + roles = HBaseReadWrite.getInstance().getUserRoles(user2); Assert.assertEquals(2, roles.size()); roleNames = roles.toArray(new String[roles.size()]); Arrays.sort(roleNames); @@ -847,13 +848,13 @@ public void userToRoleMap() throws Exception { // user1 should still have both roles since she was granted into role1 specifically. user2 // should only have role2 now since role2 was revoked from role1. - roles = HBaseReadWrite.getInstance(conf).getUserRoles(user1); + roles = HBaseReadWrite.getInstance().getUserRoles(user1); Assert.assertEquals(2, roles.size()); roleNames = roles.toArray(new String[roles.size()]); Arrays.sort(roleNames); Assert.assertArrayEquals(new String[]{roleName1, roleName2}, roleNames); - roles = HBaseReadWrite.getInstance(conf).getUserRoles(user2); + roles = HBaseReadWrite.getInstance().getUserRoles(user2); Assert.assertEquals(1, roles.size()); Assert.assertEquals(roleName1, roles.get(0)); } @@ -882,11 +883,12 @@ public void userToRoleMapOnDrop() throws Exception { store.removeRole(roleName2); - roles = HBaseReadWrite.getInstance(conf).getUserRoles(user1); + HBaseReadWrite.setConf(conf); + roles = HBaseReadWrite.getInstance().getUserRoles(user1); Assert.assertEquals(1, roles.size()); Assert.assertEquals(roleName1, roles.get(0)); - roles = HBaseReadWrite.getInstance(conf).getUserRoles(user2); + roles = HBaseReadWrite.getInstance().getUserRoles(user2); Assert.assertEquals(1, roles.size()); Assert.assertEquals(roleName1, roles.get(0)); } diff --git itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/hbase/TestStorageDescriptorSharing.java itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/hbase/TestStorageDescriptorSharing.java index decfa4a..fa7273e 100644 --- itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/hbase/TestStorageDescriptorSharing.java +++ itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/hbase/TestStorageDescriptorSharing.java @@ -97,7 +97,7 @@ public void createManyPartitions() throws Exception { Assert.assertEquals("file:/tmp/pc=" + val, p.getSd().getLocation()); } - Assert.assertEquals(1, HBaseReadWrite.getInstance(conf).countStorageDescriptor()); + Assert.assertEquals(1, HBaseReadWrite.getInstance().countStorageDescriptor()); String tableName2 = "differentTable"; sd = new StorageDescriptor(cols, "file:/tmp", "input2", "output", false, 0, @@ -106,11 +106,11 @@ public void createManyPartitions() throws Exception { emptyParameters, null, null, null); store.createTable(table); - Assert.assertEquals(2, HBaseReadWrite.getInstance(conf).countStorageDescriptor()); + Assert.assertEquals(2, HBaseReadWrite.getInstance().countStorageDescriptor()); // Drop one of the partitions and make sure it doesn't drop the storage descriptor store.dropPartition(dbName, tableName, Arrays.asList(partVals.get(0))); - Assert.assertEquals(2, HBaseReadWrite.getInstance(conf).countStorageDescriptor()); + Assert.assertEquals(2, HBaseReadWrite.getInstance().countStorageDescriptor()); // Alter the second table in a few ways to make sure it changes it's descriptor properly table = store.getTable(dbName, tableName2); @@ -119,7 +119,7 @@ public void createManyPartitions() throws Exception { // Alter the table without touching the storage descriptor table.setLastAccessTime(startTime + 1); store.alterTable(dbName, tableName2, table); - Assert.assertEquals(2, HBaseReadWrite.getInstance(conf).countStorageDescriptor()); + Assert.assertEquals(2, HBaseReadWrite.getInstance().countStorageDescriptor()); table = store.getTable(dbName, tableName2); byte[] alteredHash = HBaseUtils.hashStorageDescriptor(table.getSd(), md); Assert.assertArrayEquals(sdHash, alteredHash); @@ -127,7 +127,7 @@ public void createManyPartitions() throws Exception { // Alter the table, changing the storage descriptor table.getSd().setOutputFormat("output_changed"); store.alterTable(dbName, tableName2, table); - Assert.assertEquals(2, HBaseReadWrite.getInstance(conf).countStorageDescriptor()); + Assert.assertEquals(2, HBaseReadWrite.getInstance().countStorageDescriptor()); table = store.getTable(dbName, tableName2); alteredHash = HBaseUtils.hashStorageDescriptor(table.getSd(), md); Assert.assertFalse(Arrays.equals(sdHash, alteredHash)); @@ -137,7 +137,7 @@ public void createManyPartitions() throws Exception { sdHash = HBaseUtils.hashStorageDescriptor(part.getSd(), md); part.setLastAccessTime(part.getLastAccessTime() + 1); store.alterPartition(dbName, tableName, Arrays.asList(partVals.get(1)), part); - Assert.assertEquals(2, HBaseReadWrite.getInstance(conf).countStorageDescriptor()); + Assert.assertEquals(2, HBaseReadWrite.getInstance().countStorageDescriptor()); part = store.getPartition(dbName, tableName, Arrays.asList(partVals.get(1))); alteredHash = HBaseUtils.hashStorageDescriptor(part.getSd(), md); Assert.assertArrayEquals(sdHash, alteredHash); @@ -145,7 +145,7 @@ public void createManyPartitions() throws Exception { // Alter the partition, changing the storage descriptor part.getSd().setOutputFormat("output_changed_some_more"); store.alterPartition(dbName, tableName, Arrays.asList(partVals.get(1)), part); - Assert.assertEquals(3, HBaseReadWrite.getInstance(conf).countStorageDescriptor()); + Assert.assertEquals(3, HBaseReadWrite.getInstance().countStorageDescriptor()); part = store.getPartition(dbName, tableName, Arrays.asList(partVals.get(1))); alteredHash = HBaseUtils.hashStorageDescriptor(part.getSd(), md); Assert.assertFalse(Arrays.equals(sdHash, alteredHash)); @@ -161,7 +161,7 @@ public void createManyPartitions() throws Exception { listPartVals.add(Arrays.asList(pv)); } store.alterPartitions(dbName, tableName, listPartVals, parts); - Assert.assertEquals(3, HBaseReadWrite.getInstance(conf).countStorageDescriptor()); + Assert.assertEquals(3, HBaseReadWrite.getInstance().countStorageDescriptor()); parts = store.getPartitions(dbName, tableName, -1); alteredHash = HBaseUtils.hashStorageDescriptor(parts.get(1).getSd(), md); Assert.assertArrayEquals(sdHash, alteredHash); @@ -173,7 +173,7 @@ public void createManyPartitions() throws Exception { parts.get(i).getSd().setOutputFormat("yet_a_different_of"); } store.alterPartitions(dbName, tableName, listPartVals, parts); - Assert.assertEquals(4, HBaseReadWrite.getInstance(conf).countStorageDescriptor()); + Assert.assertEquals(4, HBaseReadWrite.getInstance().countStorageDescriptor()); parts = store.getPartitions(dbName, tableName, -1); alteredHash = HBaseUtils.hashStorageDescriptor(parts.get(1).getSd(), md); Assert.assertFalse(Arrays.equals(sdHash, alteredHash)); @@ -184,7 +184,7 @@ public void createManyPartitions() throws Exception { store.dropTable(dbName, tableName); store.dropTable(dbName, tableName2); - Assert.assertEquals(0, HBaseReadWrite.getInstance(conf).countStorageDescriptor()); + Assert.assertEquals(0, HBaseReadWrite.getInstance().countStorageDescriptor()); } diff --git itests/util/src/main/java/org/apache/hadoop/hive/metastore/hbase/HBaseStoreTestUtil.java itests/util/src/main/java/org/apache/hadoop/hive/metastore/hbase/HBaseStoreTestUtil.java index 1f42007..21e8f7e 100644 --- itests/util/src/main/java/org/apache/hadoop/hive/metastore/hbase/HBaseStoreTestUtil.java +++ itests/util/src/main/java/org/apache/hadoop/hive/metastore/hbase/HBaseStoreTestUtil.java @@ -18,14 +18,14 @@ */ package org.apache.hadoop.hive.metastore.hbase; -import java.util.List; - import org.apache.hadoop.hbase.HColumnDescriptor; import org.apache.hadoop.hbase.HTableDescriptor; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.client.HBaseAdmin; import org.apache.hadoop.hive.conf.HiveConf; +import java.util.List; + public class HBaseStoreTestUtil { public static void initHBaseMetastore(HBaseAdmin admin, HiveConf conf) throws Exception { for (String tableName : HBaseReadWrite.tableNames) { @@ -39,7 +39,7 @@ public static void initHBaseMetastore(HBaseAdmin admin, HiveConf conf) throws Ex } admin.close(); if (conf != null) { - HBaseReadWrite.getInstance(conf); + HBaseReadWrite.setConf(conf); } } } \ No newline at end of file diff --git metastore/src/java/org/apache/hadoop/hive/metastore/hbase/HBaseReadWrite.java metastore/src/java/org/apache/hadoop/hive/metastore/hbase/HBaseReadWrite.java index 951c081..781f562 100644 --- metastore/src/java/org/apache/hadoop/hive/metastore/hbase/HBaseReadWrite.java +++ metastore/src/java/org/apache/hadoop/hive/metastore/hbase/HBaseReadWrite.java @@ -19,16 +19,14 @@ package org.apache.hadoop.hive.metastore.hbase; import com.google.common.annotations.VisibleForTesting; - import org.apache.commons.codec.binary.Base64; -import org.apache.commons.lang.ArrayUtils; import org.apache.commons.lang.StringUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hbase.Cell; import org.apache.hadoop.hbase.HBaseConfiguration; import org.apache.hadoop.hbase.HConstants; -import org.apache.hadoop.hbase.Cell; import org.apache.hadoop.hbase.client.Delete; import org.apache.hadoop.hbase.client.Get; import org.apache.hadoop.hbase.client.HTableInterface; @@ -41,7 +39,6 @@ import org.apache.hadoop.hbase.filter.Filter; import org.apache.hadoop.hbase.filter.RegexStringComparator; import org.apache.hadoop.hbase.filter.RowFilter; -import org.apache.hadoop.hbase.protobuf.generated.ClientProtos.MultiRequest; import org.apache.hadoop.hive.common.ObjectPair; import org.apache.hadoop.hive.conf.HiveConf; import org.apache.hadoop.hive.metastore.api.AggrStats; @@ -185,23 +182,26 @@ protected HBaseReadWrite initialValue() { boolean entireRoleTableInCache; /** - * Get the instance of HBaseReadWrite for the current thread. This is intended to be used by - * {@link org.apache.hadoop.hive.metastore.hbase.HBaseStore} since it creates the thread local - * version of this class. + * Set the configuration for all HBaseReadWrite instances. * @param configuration Configuration object - * @return thread's instance of HBaseReadWrite */ - public static HBaseReadWrite getInstance(Configuration configuration) { - staticConf = configuration; - return self.get(); + public static synchronized void setConf(Configuration configuration) { + if (staticConf == null) { + staticConf = configuration; + } else { + LOG.info("Attempt to set conf when it has already been set."); + } } /** - * Get the instance of HBaseReadWrite for the current thread. This is inteded to be used after - * the thread has been initialized. Woe betide you if that's not the case. + * Get the instance of HBaseReadWrite for the current thread. This can only be called after + * {@link #setConf} has been called. Woe betide you if that's not the case. * @return thread's instance of HBaseReadWrite */ static HBaseReadWrite getInstance() { + if (staticConf == null) { + throw new RuntimeException("Must set conf object before getting an instance"); + } return self.get(); } diff --git metastore/src/java/org/apache/hadoop/hive/metastore/hbase/HBaseSchemaTool.java metastore/src/java/org/apache/hadoop/hive/metastore/hbase/HBaseSchemaTool.java index 1c407f1..8eb6116 100644 --- metastore/src/java/org/apache/hadoop/hive/metastore/hbase/HBaseSchemaTool.java +++ metastore/src/java/org/apache/hadoop/hive/metastore/hbase/HBaseSchemaTool.java @@ -155,7 +155,8 @@ private HBaseSchemaTool(String dbname, String tn, List pv, String fn, St roleName = rn; colNames = cn; hasStats = s; - hrw = HBaseReadWrite.getInstance(new Configuration()); + HBaseReadWrite.setConf(new Configuration()); + hrw = HBaseReadWrite.getInstance(); } public void db() throws IOException, TException { diff --git metastore/src/java/org/apache/hadoop/hive/metastore/hbase/HBaseStore.java metastore/src/java/org/apache/hadoop/hive/metastore/hbase/HBaseStore.java index 1ea25f5..09e57e5 100644 --- metastore/src/java/org/apache/hadoop/hive/metastore/hbase/HBaseStore.java +++ metastore/src/java/org/apache/hadoop/hive/metastore/hbase/HBaseStore.java @@ -2254,7 +2254,10 @@ public Configuration getConf() { } private HBaseReadWrite getHBase() { - if (hbase == null) hbase = HBaseReadWrite.getInstance(conf); + if (hbase == null) { + HBaseReadWrite.setConf(conf); + hbase = HBaseReadWrite.getInstance(); + } return hbase; } diff --git metastore/src/test/org/apache/hadoop/hive/metastore/hbase/MockUtils.java metastore/src/test/org/apache/hadoop/hive/metastore/hbase/MockUtils.java index 2198892..983129a 100644 --- metastore/src/test/org/apache/hadoop/hive/metastore/hbase/MockUtils.java +++ metastore/src/test/org/apache/hadoop/hive/metastore/hbase/MockUtils.java @@ -203,7 +203,7 @@ public Boolean answer(InvocationOnMock invocation) throws Throwable { Mockito.when(hconn.getHBaseTable(Mockito.anyString())).thenReturn(htable); HiveConf.setVar(conf, HiveConf.ConfVars.METASTORE_HBASE_CONNECTION_CLASS, HBaseReadWrite.TEST_CONN); HBaseReadWrite.setTestConnection(hconn); - HBaseReadWrite hbase = HBaseReadWrite.getInstance(conf); + HBaseReadWrite.setConf(conf); HBaseStore store = new HBaseStore(); store.setConf(conf); return store;