From 45a8ff3f49f62080a4ecf993aa2cf8b28862fa42 Mon Sep 17 00:00:00 2001 From: Ana Jalba Date: Tue, 23 Apr 2019 15:44:44 +0100 Subject: [PATCH] Allow primary metastore to have prefix (#161) --- CHANGELOG.md | 4 ++ README.md | 6 +-- .../api/model/FederatedMetaStore.java | 3 +- .../api/model/PrimaryMetaStore.java | 21 +++------ .../api/model/PrimaryMetaStoreTest.java | 6 +-- .../server/FederatedHMSHandler.java | 11 ++++- .../server/FederatedHMSHandlerTest.java | 21 ++++++++- .../bdp/waggledance/WaggleDanceRunner.java | 5 +++ .../WaggleDanceIntegrationTest.java | 45 ++++++++++++++++--- 9 files changed, 89 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 50d3b466d..8eb86f142 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +## TBD +### Changed +* Allow primary metastore to have a prefix. See [#152](https://github.com/HotelsDotCom/waggle-dance/issues/152). + ## [3.2.0] - 2019-03-27 ### Added * Configurable `latency` for each metastore in a Waggle Dance configuration. diff --git a/README.md b/README.md index 13a82be12..b7bac694b 100644 --- a/README.md +++ b/README.md @@ -141,7 +141,7 @@ The table below describes all the available configuration values for Waggle Danc | `primary-meta-store` | No | Primary MetaStore config. Can be empty but it is advised to configure it. | | `primary-meta-store.remote-meta-store-uris` | Yes | Thrift URIs of the federated read-only metastore. | | `primary-meta-store.name` | Yes | Database name that uniquely identifies this metastore. Used internally. Cannot be empty. | -| `primary-meta-store.database-prefix` | No | This will be ignored for the primary metastore and an empty string will always be used instead. | +| `primary-meta-store.database-prefix` | No | Prefix used to access the primary metastore and differentiate databases in it from databases in another metastore. The default prefix (i.e. if this value isn't explicitly set) is empty string.| | `primary-meta-store.access-control-type` | No | Sets how the client access controls should be handled. Default is `READ_ONLY` Other options `READ_AND_WRITE_AND_CREATE`, `READ_AND_WRITE_ON_DATABASE_WHITELIST` and `READ_AND_WRITE_AND_CREATE_ON_DATABASE_WHITELIST` see Access Control section below. | | `primary-meta-store.writable-database-white-list` | No | White-list of databases used to verify write access used in conjunction with `primary-meta-store.access-control-type`. The list of databases should be listed without any `primary-meta-store.database-prefix`. This property supports both full database names and (case-insensitive) [Java RegEx patterns](https://docs.oracle.com/javase/7/docs/api/java/util/regex/Pattern.html).| | `primary-meta-store.metastore-tunnel` | No | See metastore tunnel configuration values below. | @@ -356,7 +356,7 @@ database is encountered that is not prefixed then the primary metastore is used remote-meta-store-uris: thrift://primaryLocalMetastore:9083 federated-meta-stores: - name: federated - prefix: waggle_prod_ + database-prefix: waggle_prod_ remote-meta-store-uris: thrift://federatedProdMetastore:9083 Note: When choosing a prefix ensure that it does not match the start of _any_ existing database names in any of the configured metastores. To illustrate the problem this would cause, @@ -378,7 +378,7 @@ In `PREFIXED` mode any databases that are created while Waggle Dance is running remote-meta-store-uris: thrift://primaryLocalMetastore:9083 federated-meta-stores: - name: federated - prefix: waggle_prod_ + database-prefix: waggle_prod_ remote-meta-store-uris: thrift://federatedProdMetastore:9083 mapped-databases: - etldata diff --git a/waggle-dance-api/src/main/java/com/hotels/bdp/waggledance/api/model/FederatedMetaStore.java b/waggle-dance-api/src/main/java/com/hotels/bdp/waggledance/api/model/FederatedMetaStore.java index 85aed9534..8bf27ed4d 100644 --- a/waggle-dance-api/src/main/java/com/hotels/bdp/waggledance/api/model/FederatedMetaStore.java +++ b/waggle-dance-api/src/main/java/com/hotels/bdp/waggledance/api/model/FederatedMetaStore.java @@ -18,10 +18,9 @@ import java.util.Collections; import java.util.List; +import javax.validation.constraints.NotBlank; import javax.validation.constraints.NotNull; -import org.hibernate.validator.constraints.NotBlank; - public class FederatedMetaStore extends AbstractMetaStore { private @NotNull List mappedDatabases = Collections.emptyList(); diff --git a/waggle-dance-api/src/main/java/com/hotels/bdp/waggledance/api/model/PrimaryMetaStore.java b/waggle-dance-api/src/main/java/com/hotels/bdp/waggledance/api/model/PrimaryMetaStore.java index 9328dc4de..cec7cd149 100644 --- a/waggle-dance-api/src/main/java/com/hotels/bdp/waggledance/api/model/PrimaryMetaStore.java +++ b/waggle-dance-api/src/main/java/com/hotels/bdp/waggledance/api/model/PrimaryMetaStore.java @@ -15,19 +15,13 @@ */ package com.hotels.bdp.waggledance.api.model; -import javax.validation.constraints.NotNull; -import javax.validation.constraints.Size; - import java.util.Arrays; import java.util.List; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +import javax.validation.constraints.NotNull; public class PrimaryMetaStore extends AbstractMetaStore { - private final static Logger LOG = LoggerFactory.getLogger(PrimaryMetaStore.class); - private static final String EMPTY_PREFIX = ""; public PrimaryMetaStore() {} @@ -53,17 +47,14 @@ public FederationType getFederationType() { return FederationType.PRIMARY; } - @Size(min = 0, max = 0) @NotNull @Override public String getDatabasePrefix() { - // primary is always empty - return EMPTY_PREFIX; + String prefix = super.getDatabasePrefix(); + if (prefix == null) { + prefix = EMPTY_PREFIX; + } + return prefix; } - @Override - public void setDatabasePrefix(String databasePrefix) { - LOG.warn("Ignoring attempt to set prefix to '{}', the prefix for a primary metastore is always empty", - databasePrefix); - } } diff --git a/waggle-dance-api/src/test/java/com/hotels/bdp/waggledance/api/model/PrimaryMetaStoreTest.java b/waggle-dance-api/src/test/java/com/hotels/bdp/waggledance/api/model/PrimaryMetaStoreTest.java index b9aad237f..afe1b480d 100644 --- a/waggle-dance-api/src/test/java/com/hotels/bdp/waggledance/api/model/PrimaryMetaStoreTest.java +++ b/waggle-dance-api/src/test/java/com/hotels/bdp/waggledance/api/model/PrimaryMetaStoreTest.java @@ -80,11 +80,11 @@ public void nullDatabasePrefix() { @Test public void nonEmptyDatabasePrefix() { - metaStore.setDatabasePrefix("abc"); + String prefix = "abc"; + metaStore.setDatabasePrefix(prefix); Set> violations = validator.validate(metaStore); - // Violation is not triggered cause EMPTY STRING is always returned. Warning is logged instead assertThat(violations.size(), is(0)); - assertThat(metaStore.getDatabasePrefix(), is("")); + assertThat(metaStore.getDatabasePrefix(), is(prefix)); } @Test diff --git a/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/server/FederatedHMSHandler.java b/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/server/FederatedHMSHandler.java index 44a9148c8..c9f86ea14 100644 --- a/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/server/FederatedHMSHandler.java +++ b/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/server/FederatedHMSHandler.java @@ -1509,8 +1509,15 @@ public void flushCache() throws TException { @Override @Loggable(value = Loggable.DEBUG, skipResult = true, name = INVOCATION_LOG_NAME) - public GetAllFunctionsResponse get_all_functions() throws MetaException, TException { - return getPrimaryClient().get_all_functions(); + public GetAllFunctionsResponse get_all_functions() throws TException { + DatabaseMapping mapping = databaseMappingService.primaryDatabaseMapping(); + GetAllFunctionsResponse allFunctions = mapping.getClient().get_all_functions(); + if(allFunctions.getFunctions() != null) { + for (Function function : allFunctions.getFunctions()) { + mapping.transformOutboundFunction(function); + } + } + return allFunctions; } @Override diff --git a/waggle-dance-core/src/test/java/com/hotels/bdp/waggledance/server/FederatedHMSHandlerTest.java b/waggle-dance-core/src/test/java/com/hotels/bdp/waggledance/server/FederatedHMSHandlerTest.java index 6b39fe59a..f9f235eae 100644 --- a/waggle-dance-core/src/test/java/com/hotels/bdp/waggledance/server/FederatedHMSHandlerTest.java +++ b/waggle-dance-core/src/test/java/com/hotels/bdp/waggledance/server/FederatedHMSHandlerTest.java @@ -43,6 +43,7 @@ import org.apache.hadoop.hive.metastore.api.EnvironmentContext; import org.apache.hadoop.hive.metastore.api.ForeignKeysRequest; import org.apache.hadoop.hive.metastore.api.ForeignKeysResponse; +import org.apache.hadoop.hive.metastore.api.Function; import org.apache.hadoop.hive.metastore.api.GetAllFunctionsResponse; import org.apache.hadoop.hive.metastore.api.GetTableRequest; import org.apache.hadoop.hive.metastore.api.GetTableResult; @@ -766,13 +767,31 @@ public void flushCache() throws TException { } @Test - public void get_all_functions() throws MetaException, TException { + public void null_get_all_functions() throws TException { GetAllFunctionsResponse response = new GetAllFunctionsResponse(); when(primaryClient.get_all_functions()).thenReturn(response); GetAllFunctionsResponse result = handler.get_all_functions(); assertThat(result, is(response)); } + @Test + public void get_all_functions() throws TException { + String prefixedDatabase = "primary_" + DB_P; + Function function = new Function(); + function.setDbName(DB_P); + + GetAllFunctionsResponse response = new GetAllFunctionsResponse(); + response.setFunctions(Collections.singletonList(function)); + when(primaryClient.get_all_functions()).thenReturn(response); + + when(primaryMapping.transformOutboundFunction(function)).then(invocation -> { + function.setDbName(prefixedDatabase); + return function; + }); + GetAllFunctionsResponse result = handler.get_all_functions(); + assertThat(result.getFunctions().get(0).getDbName(), is(prefixedDatabase)); + } + @Test public void set_ugi() throws MetaException, TException { PanopticOperationHandler panopticHandler = Mockito.mock(PanopticOperationHandler.class); diff --git a/waggle-dance-integration-tests/src/main/java/com/hotels/bdp/waggledance/WaggleDanceRunner.java b/waggle-dance-integration-tests/src/main/java/com/hotels/bdp/waggledance/WaggleDanceRunner.java index 3fb88b85f..15cd297e8 100644 --- a/waggle-dance-integration-tests/src/main/java/com/hotels/bdp/waggledance/WaggleDanceRunner.java +++ b/waggle-dance-integration-tests/src/main/java/com/hotels/bdp/waggledance/WaggleDanceRunner.java @@ -174,6 +174,11 @@ public Builder primary( return this; } + public Builder withPrimaryPrefix(String prefix) { + primaryMetaStore.setDatabasePrefix(prefix); + return this; + } + public Builder graphite(String graphiteHost, int graphitePort, String graphitePrefix, long pollInterval) { graphiteConfiguration.setHost(graphiteHost); graphiteConfiguration.setPort(graphitePort); diff --git a/waggle-dance-integration-tests/src/test/java/com/hotels/bdp/waggledance/WaggleDanceIntegrationTest.java b/waggle-dance-integration-tests/src/test/java/com/hotels/bdp/waggledance/WaggleDanceIntegrationTest.java index a3081e533..f5787c3fd 100644 --- a/waggle-dance-integration-tests/src/test/java/com/hotels/bdp/waggledance/WaggleDanceIntegrationTest.java +++ b/waggle-dance-integration-tests/src/test/java/com/hotels/bdp/waggledance/WaggleDanceIntegrationTest.java @@ -77,6 +77,7 @@ import com.hotels.hcommon.hive.metastore.client.tunnelling.MetastoreTunnel; public class WaggleDanceIntegrationTest { + private static final Logger LOG = LoggerFactory.getLogger(WaggleDanceIntegrationTest.class); private static final String LOCAL_DATABASE = "local_database"; @@ -157,7 +158,7 @@ private HiveMetaStoreClient getWaggleDanceClient() throws MetaException { return new HiveMetaStoreClient(conf); } - private void runWaggleDance(final WaggleDanceRunner runner) throws Exception { + private void runWaggleDance(WaggleDanceRunner runner) throws Exception { executor.submit(new Runnable() { @Override public void run() { @@ -223,11 +224,42 @@ public void usePrefix() throws Exception { // Remote table String waggledRemoteDbName = PREFIXED_REMOTE_DATABASE; assertTypicalRemoteTable(proxy, waggledRemoteDbName); + } + + @Test + public void usePrimaryPrefix() throws Exception { + String primaryPrefix = "primary_"; + runner = WaggleDanceRunner + .builder(configLocation) + .databaseResolution(DatabaseResolution.PREFIXED) + .primary("primary", localServer.getThriftConnectionUri(), READ_ONLY) + .withPrimaryPrefix(primaryPrefix) + .federate(SECONDARY_METASTORE_NAME, remoteServer.getThriftConnectionUri()) + .build(); + + runWaggleDance(runner); + HiveMetaStoreClient proxy = getWaggleDanceClient(); + // Local table + String waggledLocalDbName = primaryPrefix + LOCAL_DATABASE; + assertPrefixedLocalTable(proxy, waggledLocalDbName); + + // Remote table + String waggledRemoteDbName = PREFIXED_REMOTE_DATABASE; + assertTypicalRemoteTable(proxy, waggledRemoteDbName); + } + + private void assertPrefixedLocalTable(HiveMetaStoreClient proxy, String waggledLocalDbName) throws TException { + Table localTable = localServer.client().getTable(LOCAL_DATABASE, LOCAL_TABLE); + Table waggledLocalTable = proxy.getTable(waggledLocalDbName, LOCAL_TABLE); + assertThat(waggledLocalTable.getDbName(), is(waggledLocalDbName)); + assertThat(waggledLocalTable.getTableName(), is(localTable.getTableName())); + assertThat(waggledLocalTable.getSd(), is(localTable.getSd())); + assertThat(waggledLocalTable.getParameters(), is(localTable.getParameters())); + assertThat(waggledLocalTable.getPartitionKeys(), is(localTable.getPartitionKeys())); } - private void assertTypicalRemoteTable(HiveMetaStoreClient proxy, String waggledRemoteDbName) - throws MetaException, TException, NoSuchObjectException { + private void assertTypicalRemoteTable(HiveMetaStoreClient proxy, String waggledRemoteDbName) throws TException { Table remoteTable = remoteServer.client().getTable(REMOTE_DATABASE, REMOTE_TABLE); Table waggledRemoteTable = proxy.getTable(waggledRemoteDbName, REMOTE_TABLE); assertThat(waggledRemoteTable.getDbName(), is(waggledRemoteDbName)); @@ -349,7 +381,7 @@ public void federatedWritesSucceedIfReadAndWriteOnDatabaseWhiteListIsConfigured( HiveMetaStoreClient proxy = getWaggleDanceClient(); - final String waggledRemoteDbName = PREFIXED_REMOTE_DATABASE; + String waggledRemoteDbName = PREFIXED_REMOTE_DATABASE; assertTypicalRemoteTable(proxy, waggledRemoteDbName); @@ -379,7 +411,7 @@ public void federatedWritesFailIfReadAndWriteOnDatabaseWhiteListIsNotConfigured( HiveMetaStoreClient proxy = getWaggleDanceClient(); - final String waggledRemoteDbName = PREFIXED_REMOTE_DATABASE; + String waggledRemoteDbName = PREFIXED_REMOTE_DATABASE; assertTypicalRemoteTable(proxy, waggledRemoteDbName); @@ -409,7 +441,7 @@ public void federatedWritesFailIfReadAndWriteOnDatabaseWhiteListDoesNotIncludeDb HiveMetaStoreClient proxy = getWaggleDanceClient(); - final String waggledRemoteDbName = PREFIXED_REMOTE_DATABASE; + String waggledRemoteDbName = PREFIXED_REMOTE_DATABASE; assertTypicalRemoteTable(proxy, waggledRemoteDbName); @@ -716,5 +748,4 @@ public void getDatabaseFromPatternPrefixed() throws Exception { List expected = Lists.newArrayList("default", LOCAL_DATABASE, PREFIXED_REMOTE_DATABASE); assertThat(allDatabases, is(expected)); } - }