Skip to content

Conversation

@brkyvz
Copy link
Contributor

@brkyvz brkyvz commented Sep 13, 2016

What changes were proposed in this pull request?

If a user provides listeners inside the Hive Conf, the configuration for these listeners are passed to the Hive Execution Client as well. This may cause issues for two reasons:

  1. The Execution Client will actually generate garbage
  2. The listener class needs to be both in the Spark Classpath and Hive Classpath

This PR empties the listener configurations in HiveUtils.newTemporaryConfiguration so that the execution client will not contain the listener confs, but the metadata client will.

How was this patch tested?

Unit tests

@brkyvz
Copy link
Contributor Author

brkyvz commented Sep 13, 2016

cc @yhuai

assert(conf(ConfVars.METASTORE_EVENT_LISTENERS.varname) === "")
assert(conf(ConfVars.METASTORE_END_FUNCTION_LISTENERS.varname) === "")
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@yhuai
Copy link
Contributor

yhuai commented Sep 13, 2016

LGTM pending jenkins. Once jenkins passes, I will merge this to master, branch 2.0, and branch 1.6.

@brkyvz
Copy link
Contributor Author

brkyvz commented Sep 13, 2016

@yhuai This will not clearly merge into Spark 1.6 as these were in HiveContext.scala. I can create a separate patch for that one.

@yhuai
Copy link
Contributor

yhuai commented Sep 13, 2016

Yea. That will be great!

@SparkQA
Copy link

SparkQA commented Sep 13, 2016

Test build #65331 has finished for PR 15086 at commit b2de15e.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • class HiveUtilsSuite extends QueryTest with SQLTestUtils with TestHiveSingleton

@yhuai
Copy link
Contributor

yhuai commented Sep 13, 2016

Thanks. Merging to master and branch 2.0. #15087 is the backport for branch 1.6.

asfgit pushed a commit that referenced this pull request Sep 13, 2016
## What changes were proposed in this pull request?

If a user provides listeners inside the Hive Conf, the configuration for these listeners are passed to the Hive Execution Client as well. This may cause issues for two reasons:
1. The Execution Client will actually generate garbage
2. The listener class needs to be both in the Spark Classpath and Hive Classpath

This PR empties the listener configurations in `HiveUtils.newTemporaryConfiguration` so that the execution client will not contain the listener confs, but the metadata client will.

## How was this patch tested?

Unit tests

Author: Burak Yavuz <[email protected]>

Closes #15086 from brkyvz/null-listeners.

(cherry picked from commit 72edc7e)
Signed-off-by: Yin Huai <[email protected]>
@asfgit asfgit closed this in 72edc7e Sep 13, 2016
wgtmac pushed a commit to wgtmac/spark that referenced this pull request Sep 19, 2016
## What changes were proposed in this pull request?

If a user provides listeners inside the Hive Conf, the configuration for these listeners are passed to the Hive Execution Client as well. This may cause issues for two reasons:
1. The Execution Client will actually generate garbage
2. The listener class needs to be both in the Spark Classpath and Hive Classpath

This PR empties the listener configurations in `HiveUtils.newTemporaryConfiguration` so that the execution client will not contain the listener confs, but the metadata client will.

## How was this patch tested?

Unit tests

Author: Burak Yavuz <[email protected]>

Closes apache#15086 from brkyvz/null-listeners.
@brkyvz brkyvz deleted the null-listeners branch February 3, 2019 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants