HIVE-20189: Separate metastore client code into its own module

[deniskuzZ]

What changes were proposed in this pull request?

move client classes into it's own module

Why are the changes needed?

improve the structure of standalone-metastore classes

Does this PR introduce any user-facing change?

No

How was this patch tested?

jenkins

[dengzhhu653]

The new module depends on the metastore-common, which doesn't reduce the size or dependencies from metastore-common, not sure if it's ok, for the user what will benefit from the new module?

[deniskuzZ]

It's expected for a client module to depend on a common module. The client module delivers core client functionality and can be enhanced later with features like caching.
Offering a 'client' JAR aligns with common conventions and is expected to be user-friendly.
ATM we have 2 distinct cache wrappers in different Hive modules, just because there was no structure:

• org.apache.hadoop.hive.metastore.HiveClientCache
• org.apache.iceberg.hive.CachedClientPool

[deniskuzZ]

another point I don't understand, why ql and beeline modules depend on metastore-server? I think we should drop the dependency on server and move the classes into the metastore common or client.

[okumin]

I basically agree that we can potentially improve the structure in the future. We will have obvious guidelines about how to organize files.

• metastore-client: Client-specific files, which Server doesn't need
• metastore-server: Server-specific files, which Client doesn't need
• metastore-common: Common modules

another point I don't understand, why ql and beeline modules depend on metastore-server? I think we should drop the dependency on server and move the classes into the metastore common or client.

I guess ql requires metastore-server to use an embedded HMS.

[deniskuzZ]

isn't embedded HMS only used in tests?

[okumin]

No, it isn't. It can be used when metastore.thrift.uris is empty. For example, our Hive docker image(not HMS docker image) can set up a HiveServer2 without HMS. It probably uses the embedded mode that runs HMS-equivalent threads in HS2.

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

Lines 2654 to 2663 in d7b3a5b

	/**
	* Check if metastore is being used in embedded mode.
	* This utility function exists so that the logic for determining the mode is same
	* in HiveConf and HiveMetaStoreClient
	* @param msUri - metastore server uri
	* @return true if the metastore is embedded
	*/
	public static boolean isEmbeddedMetaStore(String msUri) {
	return (msUri == null) || msUri.trim().isEmpty();
	}

[okumin]

Alternatively, we can say it is for testing purposes and separate the classes from hive-exec.

[okumin]

+1

[okumin]

We may directly return it

[deniskuzZ]

changed

[ngsg]

LGTM, +1

[ngsg]

nit: I think the original import is sorted correctly, so we might not need this change.

[deniskuzZ]

reverted

[sonarqubecloud]

Quality Gate passed

Issues
1518 New issues
0 Accepted issues

Measures
1 Security Hotspot
0.0% Coverage on New Code
0.8% Duplication on New Code

See analysis details on SonarQube Cloud