Skip to content

[SPARK-34852][SQL] Close Hive session state should use withHiveState#31949

Closed
ulysses-you wants to merge 1 commit intoapache:masterfrom
ulysses-you:SPARK-34852
Closed

[SPARK-34852][SQL] Close Hive session state should use withHiveState#31949
ulysses-you wants to merge 1 commit intoapache:masterfrom
ulysses-you:SPARK-34852

Conversation

@ulysses-you
Copy link
Contributor

What changes were proposed in this pull request?

Wrap Hive sessionStae close with withHiveState

Why are the changes needed?

Some reason:

  1. Shutdown hook is invoked using different thread
  2. Hive may use metasotre client again during closing

Otherwise, we may get such expcetion with custom hive metastore version

21/03/24 13:26:18 INFO session.SessionState: Failed to remove classloaders from DataNucleus
java.lang.RuntimeException: Unable to instantiate org.apache.hadoop.hive.ql.metadata.SessionHiveMetaStoreClient
	at org.apache.hadoop.hive.metastore.MetaStoreUtils.newInstance(MetaStoreUtils.java:1654)
	at org.apache.hadoop.hive.metastore.RetryingMetaStoreClient.<init>(RetryingMetaStoreClient.java:80)
	at org.apache.hadoop.hive.metastore.RetryingMetaStoreClient.getProxy(RetryingMetaStoreClient.java:130)
	at org.apache.hadoop.hive.metastore.RetryingMetaStoreClient.getProxy(RetryingMetaStoreClient.java:101)
	at org.apache.hadoop.hive.ql.metadata.Hive.createMetaStoreClient(Hive.java:3367)
	at org.apache.hadoop.hive.ql.metadata.Hive.getMSC(Hive.java:3406)
	at org.apache.hadoop.hive.ql.metadata.Hive.getMSC(Hive.java:3386)
	at org.apache.hadoop.hive.ql.session.SessionState.unCacheDataNucleusClassLoaders(SessionState.java:1546)
	at org.apache.hadoop.hive.ql.session.SessionState.close(SessionState.java:1536)
	at org.apache.spark.sql.hive.client.HiveClientImpl.closeState(HiveClientImpl.scala:172)
	at org.apache.spark.sql.hive.client.HiveClientImpl.$anonfun$new$1(HiveClientImpl.scala:175)
	at org.apache.spark.util.SparkShutdownHook.run(ShutdownHookManager.scala:214)
	at org.apache.spark.util.SparkShutdownHookManager.$anonfun$runAll$2(ShutdownHookManager.scala:188)

Does this PR introduce any user-facing change?

No, since this not released.

How was this patch tested?

manual test.

@ulysses-you
Copy link
Contributor Author

cc @yaooqinn @cloud-fan @wangyum

@github-actions github-actions bot added the SQL label Mar 24, 2021
@SparkQA
Copy link

SparkQA commented Mar 24, 2021

Test build #136441 has started for PR 31949 at commit b0eddb2.

@yaooqinn
Copy link
Member

Seems the only line we need is Thread.currentThread().setContextClassLoader(clientLoader.classLoader)?

@ulysses-you
Copy link
Contributor Author

Seems the only line we need is Thread.currentThread().setContextClassLoader(clientLoader.classLoader)?

This can cover the exception, but it's better to use withHiveState that also set the cached Hive into shutdown thread. That can avoid hive session state re-create an another Hive in shutdown thread.

@SparkQA
Copy link

SparkQA commented Mar 24, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/41025/

@SparkQA
Copy link

SparkQA commented Mar 24, 2021

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/41025/

@yaooqinn
Copy link
Member

retest this please

@SparkQA
Copy link

SparkQA commented Mar 24, 2021

Test build #136476 has started for PR 31949 at commit b0eddb2.

@SparkQA
Copy link

SparkQA commented Mar 24, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/41060/

@SparkQA
Copy link

SparkQA commented Mar 24, 2021

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/41060/

@yaooqinn yaooqinn closed this in 9d561e6 Mar 25, 2021
@yaooqinn
Copy link
Member

+1, thanks @ulysses-you and all reviewers. Merged to master.

@ulysses-you ulysses-you deleted the SPARK-34852 branch March 25, 2021 04:00
@ulysses-you
Copy link
Contributor Author

thanks for meging !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants