Migration hive 3 pr #219

rodolfototaro · 2021-04-26T11:23:15Z

Description
The PR is a proposal for migrating to Hive3. I kept the same configurations file adding the information about the catalog in the database configuration (if missed = "hive"). I did not implement the new Hive3 methods that i collected in the class FederatedHMSHandlerHive3 that can be merged with FederatedHMSHandlerHive when done.
I used the following approach with the internal mapping unchanged:

if the dbname embed the catalog name in the call i extract it as internal_name
I use it for the mapping
but I use the complete name for forwarding the request

Any feedback is welcome, in particurlar I need a suggestion on the new methods to forward (i'm not sure that it make sense fot all of them)

Related Issues #197

# Conflicts: # pom.xml # waggle-dance-api/pom.xml # waggle-dance-boot/pom.xml # waggle-dance-core/pom.xml # waggle-dance-integration-tests/pom.xml # waggle-dance-rest/pom.xml # waggle-dance-rpm/pom.xml # waggle-dance/pom.xml

massdosage · 2021-04-27T10:40:04Z

Thanks for raising this @rtotarocuebiq . We will try take a look and give some comments later this week.

massdosage

This looks like a really good start, I've added a round of comments and I know @patduin is also going to try take a look, hopefully this week.

massdosage · 2021-05-04T10:23:46Z

pom.xml

@@ -29,21 +29,21 @@
    <module>waggle-dance-boot</module>
    <module>waggle-dance-integration-tests</module>
    <module>waggle-dance</module>
-    <module>waggle-dance-rpm</module>
+<!--    <module>waggle-dance-rpm</module>-->


I'm guessing you're having some problem building this? What's not working for you?

massdosage · 2021-05-04T10:24:34Z

pom.xml

    <junit.version>4.13.1</junit.version>
    <mockito.version>3.5.15</mockito.version>
    <dropwizard.metrics.version>3.1.5</dropwizard.metrics.version>
    <aspectj-maven-plugin.version>1.9</aspectj-maven-plugin.version>
    <aspectj.version>1.8.9</aspectj.version>
-    <beeju.version>4.0.0</beeju.version>
+    <beeju.version>5.0.0</beeju.version>


Please merge the current state of hive-3.x into your branch as this change (and some others) have been made there now.

massdosage · 2021-05-04T10:25:12Z

waggle-dance-api/src/main/java/com/hotels/bdp/waggledance/api/model/AbstractMetaStore.java

@@ -91,6 +92,16 @@ public static PrimaryMetaStore newPrimaryInstance(String name, String remoteMeta
    return new PrimaryMetaStore(name, remoteMetaStoreUris, AccessControlType.READ_ONLY);
  }

+  public String getCatalog()
+  {


Please get the formatting to match the rest of the classes (applies to the whole PR).

massdosage · 2021-05-04T10:25:54Z

waggle-dance-api/src/main/java/com/hotels/bdp/waggledance/api/model/AbstractMetaStore.java

@@ -43,6 +43,7 @@
    @Type(value = PrimaryMetaStore.class, name = "PRIMARY"),
    @Type(value = FederatedMetaStore.class, name = "FEDERATED") })
 public abstract class AbstractMetaStore {
+  private String catalog = "hive";


Is hive the default catalog name if one doesn't specifically set one?

yes: https://github.com/apache/hive/blob/master/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/Warehouse.java#L64

massdosage · 2021-05-04T10:27:39Z

...-dance-core/src/main/java/com/hotels/bdp/waggledance/mapping/model/MetaStoreMappingImpl.java

@@ -15,6 +15,8 @@
 */
 package com.hotels.bdp.waggledance.mapping.model;

+import static org.apache.hadoop.hive.metastore.utils.MetaStoreUtils.*;


Please expand to fully qualified imports instead of *.

massdosage · 2021-05-04T10:42:45Z

...ce-core/src/test/java/com/hotels/bdp/waggledance/mapping/model/MetaStoreMappingImplTest.java

@@ -58,9 +58,9 @@

  @Before
  public void init() {
-    metaStoreMapping = new MetaStoreMappingImpl(DATABASE_PREFIX, NAME, client, accessControlHandler, DIRECT, LATENCY,
+    metaStoreMapping = new MetaStoreMappingImpl(DATABASE_PREFIX, NAME,"hive", client, accessControlHandler, DIRECT, LATENCY,


Suggested change

metaStoreMapping = new MetaStoreMappingImpl(DATABASE_PREFIX, NAME,"hive", client, accessControlHandler, DIRECT, LATENCY,

metaStoreMapping = new MetaStoreMappingImpl(DATABASE_PREFIX, NAME, "hive", client, accessControlHandler, DIRECT, LATENCY,

massdosage · 2021-05-04T10:42:53Z

...ce-core/src/test/java/com/hotels/bdp/waggledance/mapping/model/MetaStoreMappingImplTest.java

        new DefaultMetaStoreFilterHookImpl(new HiveConf()));
-    tunneledMetaStoreMapping = new MetaStoreMappingImpl(DATABASE_PREFIX, NAME, client, accessControlHandler, TUNNELED,
+    tunneledMetaStoreMapping = new MetaStoreMappingImpl(DATABASE_PREFIX, NAME,"hive", client, accessControlHandler, TUNNELED,


Suggested change

tunneledMetaStoreMapping = new MetaStoreMappingImpl(DATABASE_PREFIX, NAME,"hive", client, accessControlHandler, TUNNELED,

tunneledMetaStoreMapping = new MetaStoreMappingImpl(DATABASE_PREFIX, NAME, "hive", client, accessControlHandler, TUNNELED,

massdosage · 2021-05-04T10:43:02Z

waggle-dance-core/src/test/java/com/hotels/bdp/waggledance/mapping/model/PrefixMappingTest.java

@@ -27,7 +27,7 @@
 @SuppressWarnings("resource")
 public class PrefixMappingTest {

-  private final MetaStoreMapping metaStoreMapping = new MetaStoreMappingImpl("prefix_", "mapping", null, null, DIRECT,
+  private final MetaStoreMapping metaStoreMapping = new MetaStoreMappingImpl("prefix_", "mapping","hive", null, null, DIRECT,


Suggested change

private final MetaStoreMapping metaStoreMapping = new MetaStoreMappingImpl("prefix_", "mapping","hive", null, null, DIRECT,

private final MetaStoreMapping metaStoreMapping = new MetaStoreMappingImpl("prefix_", "mapping", "hive", null, null, DIRECT,

massdosage · 2021-05-04T10:44:06Z

waggle-dance-integration-tests/src/main/java/com/hotels/bdp/waggledance/WaggleDanceRunner.java

@@ -38,6 +36,7 @@
 import org.apache.commons.vfs2.FileSystemException;
 import org.apache.commons.vfs2.FileSystemManager;
 import org.apache.commons.vfs2.VFS;
+import org.apache.logging.log4j.util.Strings;


I don't think we should be using this Strings util class but rather one from guava or apache-commons

massdosage · 2021-05-04T10:44:56Z

...e-integration-tests/src/test/java/com/hotels/bdp/waggledance/WaggleDanceIntegrationTest.java

-  public @Rule ThriftHiveMetaStoreJUnitRule localServer = new ThriftHiveMetaStoreJUnitRule(LOCAL_DATABASE);
-  public @Rule ThriftHiveMetaStoreJUnitRule remoteServer = new ThriftHiveMetaStoreJUnitRule(REMOTE_DATABASE);
-  public @Rule ThriftHiveMetaStoreJUnitRule newRemoteServer = new ThriftHiveMetaStoreJUnitRule();
+  public static Map<String,String> hiveConfig = new HashMap<>();


With the move to BeeJU 5.x which supports Hive-3.x I'm not sure we need to do this, it should be doing this itself?

patduin · 2021-05-13T09:50:54Z

waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/server/FederatedHMSHandler.java

+  {
+    try {
+      String internalName = parseDbName(dbName, null)[DB_NAME];
+      return internalName!=null ? internalName : "*";


Not sure if defaulting to * is correct here, depends on what called this perhaps just return what was given, if null it's null the caller should deal with it.

patduin

Really nice, made some comments as well, please have a look, thank you for contributing!

patduin · 2023-10-02T15:02:41Z

There has been many changes and this go out of date. Closed.

rtotaro added 3 commits April 22, 2021 06:51

ExpediaGroup#197 - In Progress: hive 3 migration

340b0a6

ExpediaGroup#197 Hive 3 migration: porting existing logics

c73da63

Added mapping to internal database name in all methods

feedb71

massdosage suggested changes May 4, 2021

View reviewed changes

patduin reviewed May 13, 2021

View reviewed changes

patduin requested changes May 13, 2021

View reviewed changes

patduin closed this Oct 2, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migration hive 3 pr #219

Migration hive 3 pr #219

rodolfototaro commented Apr 26, 2021

massdosage commented Apr 27, 2021 •

edited

Loading

massdosage left a comment

massdosage May 4, 2021

massdosage May 4, 2021

massdosage May 4, 2021

massdosage May 4, 2021

patduin May 13, 2021

massdosage May 4, 2021

massdosage May 4, 2021

massdosage May 4, 2021

massdosage May 4, 2021

massdosage May 4, 2021

massdosage May 4, 2021

patduin May 13, 2021

patduin left a comment

patduin commented Oct 2, 2023

	metaStoreMapping = new MetaStoreMappingImpl(DATABASE_PREFIX, NAME,"hive", client, accessControlHandler, DIRECT, LATENCY,
	metaStoreMapping = new MetaStoreMappingImpl(DATABASE_PREFIX, NAME, "hive", client, accessControlHandler, DIRECT, LATENCY,

	tunneledMetaStoreMapping = new MetaStoreMappingImpl(DATABASE_PREFIX, NAME,"hive", client, accessControlHandler, TUNNELED,
	tunneledMetaStoreMapping = new MetaStoreMappingImpl(DATABASE_PREFIX, NAME, "hive", client, accessControlHandler, TUNNELED,

	private final MetaStoreMapping metaStoreMapping = new MetaStoreMappingImpl("prefix_", "mapping","hive", null, null, DIRECT,
	private final MetaStoreMapping metaStoreMapping = new MetaStoreMappingImpl("prefix_", "mapping", "hive", null, null, DIRECT,

Migration hive 3 pr #219

Migration hive 3 pr #219

Conversation

rodolfototaro commented Apr 26, 2021

massdosage commented Apr 27, 2021 • edited Loading

massdosage left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

patduin left a comment

Choose a reason for hiding this comment

patduin commented Oct 2, 2023

massdosage commented Apr 27, 2021 •

edited

Loading