From 751eb17f2a4aca07b820711854a919396ea93350 Mon Sep 17 00:00:00 2001 From: Patrick Duin Date: Thu, 23 May 2019 22:08:54 +0200 Subject: [PATCH 1/3] removed check for prefix so we can fallback to empty --- CHANGELOG.md | 5 ++++ .../mapping/model/MetaStoreMappingImpl.java | 7 +++--- .../model/MetaStoreMappingImplTest.java | 6 ++--- .../WaggleDanceIntegrationTest.java | 23 ++++++++----------- 4 files changed, 20 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d0a5c0c9f..c9c4a4d0a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +## [3.3.2] - TBD +### Changed +* Changed a prefixed *primary* metastore to fallback to 'empty prefix' if nothing specified. See [#173](https://github.com/HotelsDotCom/waggle-dance/issues/173). + + ## [3.3.1] - 2019-05-20 ### Fixed * `Show Functions` now shows UDFs from all metastores. See [#164](https://github.com/HotelsDotCom/waggle-dance/issues/164). diff --git a/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/mapping/model/MetaStoreMappingImpl.java b/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/mapping/model/MetaStoreMappingImpl.java index 6f3e323d9..c6f3ca206 100644 --- a/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/mapping/model/MetaStoreMappingImpl.java +++ b/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/mapping/model/MetaStoreMappingImpl.java @@ -78,11 +78,10 @@ public Database transformOutboundDatabase(Database database) { @Override public String transformInboundDatabaseName(String databaseName) { databaseName = databaseName.toLowerCase(Locale.ROOT); - if (!databaseName.startsWith(getDatabasePrefix())) { - throw new IllegalArgumentException( - "Database '" + databaseName + "' does not start with prefix '" + getDatabasePrefix() + "'"); + if (databaseName.startsWith(getDatabasePrefix())) { + return databaseName.substring(getDatabasePrefix().length()); } - return databaseName.substring(getDatabasePrefix().length()); + return databaseName; } @Override diff --git a/waggle-dance-core/src/test/java/com/hotels/bdp/waggledance/mapping/model/MetaStoreMappingImplTest.java b/waggle-dance-core/src/test/java/com/hotels/bdp/waggledance/mapping/model/MetaStoreMappingImplTest.java index 67c28d9b1..d2af72bec 100644 --- a/waggle-dance-core/src/test/java/com/hotels/bdp/waggledance/mapping/model/MetaStoreMappingImplTest.java +++ b/waggle-dance-core/src/test/java/com/hotels/bdp/waggledance/mapping/model/MetaStoreMappingImplTest.java @@ -79,9 +79,9 @@ public void transformInboundDatabaseName() { assertThat(metaStoreMapping.transformInboundDatabaseName("Prefix_My_Database"), is("my_database")); } - @Test(expected = IllegalArgumentException.class) - public void transformInboundDatabaseNameFails() { - metaStoreMapping.transformInboundDatabaseName("waggle_database"); + @Test + public void transformInboundDatabaseNameWithoutPrefixReturnsDatabase() { + assertThat(metaStoreMapping.transformInboundDatabaseName("no_prefix_My_Database"), is("no_prefix_my_database")); } @Test 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 e88b7b2d0..d8ce51495 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 @@ -276,22 +276,17 @@ public void usePrimaryPrefix() throws Exception { HiveMetaStoreClient proxy = getWaggleDanceClient(); // Local table - String waggledLocalDbName = primaryPrefix + LOCAL_DATABASE; - assertPrefixedLocalTable(proxy, waggledLocalDbName); + String prefixedLocalDbName = primaryPrefix + LOCAL_DATABASE; + Table localTable = proxy.getTable(prefixedLocalDbName, LOCAL_TABLE); + assertThat(localTable.getDbName(), is(prefixedLocalDbName)); - // Remote table - String waggledRemoteDbName = PREFIXED_REMOTE_DATABASE; - assertTypicalRemoteTable(proxy, waggledRemoteDbName); - } + // fetch without prefix works and result is prefixed + Table localTable2 = proxy.getTable(LOCAL_DATABASE, LOCAL_TABLE); + assertThat(localTable2.getDbName(), is(prefixedLocalDbName)); - 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())); + // Remote table + String prefixedRemoteDbName = PREFIXED_REMOTE_DATABASE; + assertTypicalRemoteTable(proxy, prefixedRemoteDbName); } private void assertTypicalRemoteTable(HiveMetaStoreClient proxy, String waggledRemoteDbName) throws TException { From 6f6ef30a964463723b7ae3040190bc29444158c3 Mon Sep 17 00:00:00 2001 From: Patrick Duin Date: Fri, 24 May 2019 17:13:16 +0200 Subject: [PATCH 2/3] formatting --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c9c4a4d0a..e2d4a09da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,6 @@ ### Changed * Changed a prefixed *primary* metastore to fallback to 'empty prefix' if nothing specified. See [#173](https://github.com/HotelsDotCom/waggle-dance/issues/173). - ## [3.3.1] - 2019-05-20 ### Fixed * `Show Functions` now shows UDFs from all metastores. See [#164](https://github.com/HotelsDotCom/waggle-dance/issues/164). From 40c481144410e8f925575abd1bbde949aca484ed Mon Sep 17 00:00:00 2001 From: Patrick Duin Date: Thu, 6 Jun 2019 14:38:02 +0200 Subject: [PATCH 3/3] Added logic to return unprefixed UDFs when primary metastore is using prefixes for seemless fallback --- .../PrefixBasedDatabaseMappingService.java | 31 +++++++++++++++++++ ...PrefixBasedDatabaseMappingServiceTest.java | 28 +++++++++++++++++ .../WaggleDanceIntegrationTest.java | 11 ++++--- 3 files changed, 66 insertions(+), 4 deletions(-) diff --git a/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/mapping/service/impl/PrefixBasedDatabaseMappingService.java b/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/mapping/service/impl/PrefixBasedDatabaseMappingService.java index 5aacb67a8..f91ed9517 100644 --- a/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/mapping/service/impl/PrefixBasedDatabaseMappingService.java +++ b/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/mapping/service/impl/PrefixBasedDatabaseMappingService.java @@ -31,6 +31,8 @@ import javax.validation.constraints.NotNull; import org.apache.commons.io.IOUtils; +import org.apache.hadoop.hive.metastore.api.Function; +import org.apache.hadoop.hive.metastore.api.GetAllFunctionsResponse; import org.apache.hadoop.hive.metastore.api.TableMeta; import org.apache.logging.log4j.util.Strings; import org.slf4j.Logger; @@ -304,6 +306,35 @@ public List getAllDatabases() { return result; } + @Override + public GetAllFunctionsResponse getAllFunctions(List databaseMappings) { + GetAllFunctionsResponse allFunctions = super.getAllFunctions(databaseMappings); + addNonPrefixedPrimaryMetastoreFunctions(allFunctions); + return allFunctions; + } + + /* + * This is done to ensure we can fallback to un-prefixed UDFs (for primary Metastore only). + */ + private void addNonPrefixedPrimaryMetastoreFunctions(GetAllFunctionsResponse allFunctions) { + List newFunctions = new ArrayList<>(); + String primaryPrefix = primaryDatabaseMapping().getDatabasePrefix(); + if (!"".equals(primaryPrefix)) { + if (allFunctions.isSetFunctions()) { + for (Function function : allFunctions.getFunctions()) { + newFunctions.add(function); + if (function.getDbName().startsWith(primaryPrefix)) { + Function unprefixed = new Function(function); + // strip off the prefix + primaryDatabaseMapping.transformInboundFunction(unprefixed); + newFunctions.add(unprefixed); + } + } + allFunctions.setFunctions(newFunctions); + } + } + } + @Override protected PanopticOperationExecutor getPanopticOperationExecutor() { return new PanopticConcurrentOperationExecutor(); diff --git a/waggle-dance-core/src/test/java/com/hotels/bdp/waggledance/mapping/service/impl/PrefixBasedDatabaseMappingServiceTest.java b/waggle-dance-core/src/test/java/com/hotels/bdp/waggledance/mapping/service/impl/PrefixBasedDatabaseMappingServiceTest.java index 880de9f29..550435549 100644 --- a/waggle-dance-core/src/test/java/com/hotels/bdp/waggledance/mapping/service/impl/PrefixBasedDatabaseMappingServiceTest.java +++ b/waggle-dance-core/src/test/java/com/hotels/bdp/waggledance/mapping/service/impl/PrefixBasedDatabaseMappingServiceTest.java @@ -363,6 +363,34 @@ public void panopticOperationsHandlerGetAllFunctions() throws Exception { assertThat(result.getFunctions().get(1).getFunctionName(), is("fn2")); } + @Test + public void panopticOperationsHandlerGetAllFunctionsPrimaryMappingHasPrefix() throws Exception { + when(metaStoreMappingPrimary.getDatabasePrefix()).thenReturn("prefixed_"); + when(metaStoreMappingPrimary.transformOutboundDatabaseName("db")).thenReturn("prefixed_db"); + when(metaStoreMappingPrimary.transformInboundDatabaseName("prefixed_db")).thenReturn("db"); + GetAllFunctionsResponse responsePrimary = new GetAllFunctionsResponse(); + responsePrimary.addToFunctions(newFunction("db", "fn1")); + when(primaryDatabaseClient.get_all_functions()).thenReturn(responsePrimary); + + when(metaStoreMappingFederated.transformOutboundDatabaseName("db")).thenReturn(DB_PREFIX + "db"); + when(metaStoreMappingFederated.getClient()).thenReturn(federatedDatabaseClient); + GetAllFunctionsResponse responseFederated = new GetAllFunctionsResponse(); + responseFederated.addToFunctions(newFunction("db", "fn2")); + when(federatedDatabaseClient.get_all_functions()).thenReturn(responseFederated); + + service = new PrefixBasedDatabaseMappingService(metaStoreMappingFactory, + Arrays.asList(primaryMetastore, federatedMetastore), queryMapping); + PanopticOperationHandler handler = service.getPanopticOperationHandler(); + GetAllFunctionsResponse result = handler.getAllFunctions(service.getDatabaseMappings()); + assertThat(result.getFunctionsSize(), is(3)); + assertThat(result.getFunctions().get(0).getFunctionName(), is("fn1")); + assertThat(result.getFunctions().get(0).getDbName(), is("prefixed_db")); + assertThat(result.getFunctions().get(1).getFunctionName(), is("fn1")); + assertThat(result.getFunctions().get(1).getDbName(), is("db")); + assertThat(result.getFunctions().get(2).getFunctionName(), is("fn2")); + assertThat(result.getFunctions().get(2).getDbName(), is(DB_PREFIX + "db")); + } + @Test(expected = NoPrimaryMetastoreException.class) public void noPrimaryMappingThrowsException() { when(metaStoreMappingFactory.newInstance(federatedMetastore)).thenReturn(metaStoreMappingFederated); 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 d8ce51495..b3037ff1b 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 @@ -216,6 +216,7 @@ public void typicalGetAllFunctions() throws Exception { .builder(configLocation) .databaseResolution(DatabaseResolution.PREFIXED) .primary("primary", localServer.getThriftConnectionUri(), READ_ONLY) + .withPrimaryPrefix("primary_") .federate(SECONDARY_METASTORE_NAME, remoteServer.getThriftConnectionUri(), REMOTE_DATABASE) .build(); @@ -232,11 +233,13 @@ public void typicalGetAllFunctions() throws Exception { GetAllFunctionsResponse allFunctions = proxy.getAllFunctions(); List functions = allFunctions.getFunctions(); - assertThat(functions.size(), is(2)); + assertThat(functions.size(), is(3)); assertThat(functions.get(0).getFunctionName(), is("fn1")); - assertThat(functions.get(0).getDbName(), is(LOCAL_DATABASE)); - assertThat(functions.get(1).getFunctionName(), is("fn2")); - assertThat(functions.get(1).getDbName(), is(PREFIXED_REMOTE_DATABASE)); + assertThat(functions.get(0).getDbName(), is("primary_" + LOCAL_DATABASE)); + assertThat(functions.get(1).getFunctionName(), is("fn1")); + assertThat(functions.get(1).getDbName(), is(LOCAL_DATABASE)); + assertThat(functions.get(2).getFunctionName(), is("fn2")); + assertThat(functions.get(2).getDbName(), is(PREFIXED_REMOTE_DATABASE)); } @Test