HIVE-12679: Allow users to be able to specify an implementation of IMetaStoreClient via HiveConf #5955

ngsg · 2025-07-11T01:50:10Z

What changes were proposed in this pull request?

This patch aims to support configurable IMetaStoreClient.

This patch has been submitted three times previously. Austin originally proposed HIVE-12679 and submitted the initial patch in 2016. @moomindani later resubmitted it in #1402, and then @okumin took it over in #4444.

Why are the changes needed?

In some scenarios, users may want to run Hive as a general-purpose execution engine on top of a third-party data catalog. For instance, AWS Glue Data Catalog is an example of such usage, and there is also an open ticket to implement an Iceberg REST catalog (HIVE-28658). As noted in the previous PR (#4444), more than 40 people are watching HIVE-12679, which I believe reflects strong demand for using Hive in this more flexible architecture. Therefore, I believe it is worth completing this patch to support external data catalogs in Hive.

Does this PR introduce any user-facing change?

No, users can use the existing HMS client without any changes in their configuration files.
For the documentation fix, I will update hive-site once this patch is accepted and merged.

How was this patch tested?

This patch includes two simple tests loading MetaStoreClient via HiveMetaStoreClientFactory.

Note for those familiar with the earlier patch:

Regarding the design decision about MetaStoreClientFactory:
This patch does not support a customizable MSC factory as the original patch did; instead, it supports a customizable MSC. In other words, this patch introduces metastore.client.class, not metastore.client.factory.class.
I made this change not because I preferred this approach, but because I followed the majority opinion in the discussion here. Personally, I think both approaches are valid and have their merits, and I do not have a strong opinion among them. I would appreciate it if you could share your thoughts on this decision.
Regarding RetryingMetaStoreClient and other classes that instantiate MetaStoreClient (link):
After resolving HIVE-27473, Hive now creates ThriftHiveMetaStoreClient only in Hive.java and HiveMetaStoreClient. Therefore, I believe it is sufficient to replace these two call sites with the new HiveMetaStoreClientFactory.

deniskuzZ · 2025-07-11T05:46:26Z

Hi @ngsg, let's get #5924 in first. Please review it when time allows.
Thank you!

okumin · 2025-07-13T05:22:39Z

...common/src/main/java/org/apache/hadoop/hive/metastore/client/HiveMetaStoreClientFactory.java

+  private static final Logger LOG = LoggerFactory.getLogger(HiveMetaStoreClientFactory.class);
+
+  public static IMetaStoreClient newClient(Configuration conf, boolean allowEmbedded) throws MetaException {
+    String mscClassName = MetastoreConf.getVar(conf, MetastoreConf.ConfVars.METASTORE_CLIENT_CLASS);


Can we use MetastoreConf.getClass?

Sure, I'll modify it to use getClass once HIVE-20189 is merged.

okumin · 2025-07-13T05:27:05Z

...tore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java

+    METASTORE_CLIENT_CLASS("metastore.client.class",
+        "hive.metastore.client.class",
+        "org.apache.hadoop.hive.metastore.client.ThriftHiveMetaStoreClient",
+        "The name of MetaStoreClient class that implements the IMetaStoreClient interface."),


@moomindani Please feel free to give us a suggestion if you have any thoughts.

Regarding the design decision about MetaStoreClientFactory:
This patch does not support a customizable MSC factory as the original patch did; instead, it supports a customizable MSC. In other words, this patch introduces metastore.client.class, not metastore.client.factory.class.
I made this change not because I preferred this approach, but because I followed the majority opinion in the discussion #4444 (review). Personally, I think both approaches are valid and have their merits, and I do not have a strong opinion among them. I would appreciate it if you could share your thoughts on this decision.

Thanks, LGTM, I have no concerns around this.

I have a concern here: if we specify the client via the configuration, it means each time we are allowed to query one type of metadata only. For example, we set the client to Glue for the tables stored in Glue, if we need the Hive tables later on in the same session, we should reset the client to Hive, and just think loud, if the SQL contains the Hive and Glue table?

From my point of view, I would suggest the pattern(the database or table) or the catalog routed way to choose which the client will use to obtain the metadata.

From my point of view, I would suggest the pattern(the database or table) or the catalog routed way to choose which the client will use to obtain the metadata.

I believe this is somewhat similar to the design proposed in HIVE-28879 (Federated Catalog).
From my perspective, I see HIVE-12679 as a milestone for both HIVE-28658 (Iceberg REST Catalog, #5628) and HIVE-28879. So, I'm fine with moving directly to HIVE-28879 or pursuing any other approach, as long as we aim to support third-party data catalogs. However, since this ticket predates my contributions and has many watchers, I want to ask for and respect others' opinions as well.

@ngsg maybe METASTORE_CLIENT_IMPL, similar to RAW_STORE_IMPL?

how about a new property to configure those mappings? e.g, metastore.client.meta.class.mappings=catalog:client_classname,catalog.db(or db_pattern):client_classname,catalog.db.table:client_classname

I don't think we need multiple client impl for the same catalog (aka Snowflake, Glue, HMS, Rest, etc)
Instead, I’d suggest using a simple comma-separated list. Later, that would be persisted to the backend database during the Catalog registration (HIVE-26227)

METASTORE_CLIENT_IMPL("metastore.client.impl", catalog:client_classname)

Maybe we should address this in HIVE-28879, since we also need to provide connection details per catalog

@ngsg maybe METASTORE_CLIENT_IMPL, similar to RAW_STORE_IMPL?

I changed METASTORE_CLINT_CLASS to METASTORE_CLIENT_IMPL and metastore.client.class to metastore.client.impl.

how about a new property to configure those mappings? e.g, metastore.client.meta.class.mappings=catalog:client_classname,catalog.db(or db_pattern):client_classname,catalog.db.table:client_classname

Regarding the mapping, if there were no ThriftHiveMetaStoreClient-related logic in HiveMetaStoreClient, I think I could implement a mapping from a catalog to the actual client. However, due to that logic, I don't currently have a clear design to support it. Let me think more about a better design and share my thoughts once I’ve summarized them.

Another thought I had is that if we eventually implement HIVE-28879, we may not need this configuration key to specify either a single class or a mapping, since the relevant information would be stored in the MetaStore backend database. Given that we likely have enough time before the next release, I'm unsure about the feasibility of introducing a temporary configuration key that won't appear in any release. What do you think about keeping this patch as a fallback option in case HIVE-28879 isn't ready in time for the next release, and proceeding directly with HIVE-28879 for now?

Another thought I had is that if we eventually implement HIVE-28879, we may not need this configuration key to specify either a single class or a mapping, since the relevant information would be stored in the MetaStore backend database.

Totally agree + the proposed mapping might require connection details as well. I think we should go ahead and merge this PR, and start thinking on a more generic solution like HIVE-28879

deniskuzZ · 2025-07-15T11:24:59Z

HIVE-20189 is merged, @ngsg could you please rebase

…e-common

...rc/main/java/org/apache/hadoop/hive/metastore/client/builder/HiveMetaStoreClientBuilder.java

...ore/metastore-client/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java

sonarqubecloud · 2025-07-25T06:11:06Z

Quality Gate passed

Issues
23 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

deniskuzZ

+1

ayushtkn · 2025-07-30T21:06:03Z

This seems good to go? if yes, @ngsg mind hitting the merge button?

ngsg · 2025-07-31T01:17:36Z

@ayushtkn, thanks for the reminder. I'll merge it shortly.

ngsg · 2025-07-31T02:23:48Z

Thanks all for the review!

I have read the guide and tried to follow the documented process for merging. Since this is my first time merging, I would appreciate it if someone could check whether I've made any mistakes I might have overlooked.

ayushtkn · 2025-07-31T06:58:10Z

I have read the guide and tried to follow the documented process for merging. Since this is my first time merging, I would appreciate it if someone could check whether I've made any mistakes I might have overlooked.

@ngsg It is all good

asf-ci-hive added the tests pending label Jul 11, 2025

asf-ci-hive added tests unstable and removed tests pending labels Jul 11, 2025

okumin reviewed Jul 13, 2025

View reviewed changes

moomindani and others added 11 commits July 16, 2025 10:45

HIVE-12679: Cherry-pick 1558b1a

a943652

HIVE-12679: Resolve conflict

5883c9a

HIVE-12679: Follow warning of IDE

c132475

HIVE-12679: Move the parameter to MetastoreConf

f145e1d

Remove the param from HiveConf

33a5637

maybe compile ok

7dcf6c0

Option1: Use metastore.client.factory.class

5712a01

Option 2: Use metastore.client.class

156e00f

Move HiveMetaStoreClientFactory from hive-exec to standalone-metastor…

004f13e

…e-common

minor

01421d2

compile ok

fe11b25

ngsg force-pushed the HIVE-12679-metastore-final-from-okumin branch from 605e7a6 to fe11b25 Compare July 16, 2025 05:31

asf-ci-hive added tests pending tests passed and removed tests unstable tests pending labels Jul 16, 2025

deniskuzZ reviewed Jul 23, 2025

View reviewed changes

...rc/main/java/org/apache/hadoop/hive/metastore/client/builder/HiveMetaStoreClientBuilder.java Outdated Show resolved Hide resolved

deniskuzZ reviewed Jul 23, 2025

View reviewed changes

...ore/metastore-client/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java Outdated Show resolved Hide resolved

deniskuzZ reviewed Jul 23, 2025

View reviewed changes

...ore/metastore-client/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java Outdated Show resolved Hide resolved

deniskuzZ reviewed Jul 23, 2025

View reviewed changes

...ore/metastore-client/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java Show resolved Hide resolved

Address review comments

166e897

asf-ci-hive added tests pending and removed tests passed labels Jul 25, 2025

asf-ci-hive added tests passed and removed tests pending labels Jul 25, 2025

deniskuzZ approved these changes Jul 25, 2025

View reviewed changes

ngsg merged commit bcfa755 into apache:master Jul 31, 2025
4 checks passed

HIVE-12679: Allow users to be able to specify an implementation of IMetaStoreClient via HiveConf #5955

HIVE-12679: Allow users to be able to specify an implementation of IMetaStoreClient via HiveConf #5955

Uh oh!

Conversation

ngsg commented Jul 11, 2025

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

deniskuzZ commented Jul 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dengzhhu653 Jul 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dengzhhu653 Jul 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ngsg Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

deniskuzZ commented Jul 15, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sonarqubecloud bot commented Jul 25, 2025

Quality Gate passed

Uh oh!

deniskuzZ left a comment

Choose a reason for hiding this comment

Uh oh!

ayushtkn commented Jul 30, 2025

Uh oh!

ngsg commented Jul 31, 2025

Uh oh!

Uh oh!

ngsg commented Jul 31, 2025

Uh oh!

ayushtkn commented Jul 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

dengzhhu653 Jul 16, 2025 •

edited

Loading

dengzhhu653 Jul 24, 2025 •

edited

Loading

ngsg Jul 25, 2025 •

edited

Loading

ayushtkn commented Jul 31, 2025 •

edited

Loading