Skip to content

[SPARK-26254][CORE] Extract Hive + Kafka dependencies from Core. #23499

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
gaborgsomogyi wants to merge 10 commits into from
Closed

[SPARK-26254][CORE] Extract Hive + Kafka dependencies from Core. #23499

gaborgsomogyi wants to merge 10 commits into from

Conversation

@gaborgsomogyi
Copy link
Contributor

@gaborgsomogyi gaborgsomogyi commented Jan 9, 2019

What changes were proposed in this pull request?

There are ugly provided dependencies inside core for the following:

  • Hive
  • Kafka

In this PR I've extracted them out. This PR contains the following:

  • Token providers are now loaded with service loader
  • Hive token provider moved to hive project
  • Kafka token provider extracted into a new project

How was this patch tested?

Existing + newly added unit tests.
Additionally tested on cluster.

@SparkQA
Copy link

SparkQA commented Jan 9, 2019

Test build #100969 has finished for PR 23499 at commit cd7db2f.

  • This patch fails to build.
  • This patch does not merge cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jan 9, 2019

Test build #100970 has finished for PR 23499 at commit 47ea157.

  • This patch fails to build.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jan 9, 2019

Test build #100975 has finished for PR 23499 at commit d8188c0.

  • This patch fails MiMa tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@gaborgsomogyi gaborgsomogyi changed the title [SPARK-26254][CORE] Remove Hive + Kafka dependencies from Core. [SPARK-26254][CORE] Extract Hive + Kafka dependencies from Core. Jan 9, 2019
@SparkQA
Copy link

SparkQA commented Jan 9, 2019

Test build #100983 has finished for PR 23499 at commit c7a4282.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jan 9, 2019

Test build #100984 has finished for PR 23499 at commit 6e0178c.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@gaborgsomogyi
Copy link
Contributor Author

gaborgsomogyi commented Jan 10, 2019

retest this please

srowen
srowen reviewed Jan 10, 2019
View reviewed changes
@SparkQA
Copy link

SparkQA commented Jan 10, 2019

Test build #101021 has finished for PR 23499 at commit 6e0178c.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@gaborgsomogyi
Copy link
Contributor Author

gaborgsomogyi commented Jan 10, 2019 •
edited
Loading

cc @vanzin @dongjoon-hyun @HeartSaVioR

@gatorsmile
Copy link
Member

gatorsmile commented Jan 14, 2019

In Spark 3.0, shading all libraries that Spark depends is the direction we discussed in the mailing list. @gaborgsomogyi Thus, could you change your PR based on our direction? Thanks!

@vanzin
Copy link
Contributor

vanzin commented Jan 14, 2019

This is a refactoring so that spark-core doesn't depend on Hive and Kafka at compile time. It's not really related to shading, and it would be distracting to mix both in this PR.

If a decision has been made about shading everything (I don't remember a vote), then there will be a lot of work that is unrelated to this PR to get there.

@srowen
Copy link
Member

srowen commented Jan 14, 2019

(Agree that shading is a big, important, but separate change to be considered)

@gaborgsomogyi
Copy link
Contributor Author

gaborgsomogyi commented Jan 14, 2019

@gatorsmile happy to handle this problem in the streaming/kafka area in separate PRs. I would appreciate if you can point the details and the vote to pick up the topic.

@felixcheung
Copy link
Member

felixcheung commented Jan 15, 2019 via email

vanzin
vanzin reviewed Jan 18, 2019
View reviewed changes
core/src/main/scala/org/apache/spark/deploy/security/HadoopDelegationTokenProvider.scala
def obtainDelegationTokens(
hadoopConf: Configuration,
sparkConf: SparkConf,
fileSystems: Set[FileSystem],
Copy link
Contributor

@vanzin vanzin Jan 18, 2019

Choose a reason for hiding this comment

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

This is a bit odd, and ultimately I think that YarnSparkHadoopUtil.hadoopFSsToAccess should be used for everybody (a.k.a. moved to HadoopFSDelegationTokenProvider), but that should be a separate change.

Copy link
Contributor Author

@gaborgsomogyi gaborgsomogyi Jan 21, 2019

Choose a reason for hiding this comment

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

Felt the same when changed but didn't have good solution. Good suggestion, will file a jira and resolve it.

@SparkQA
Copy link

SparkQA commented Jan 21, 2019

Test build #101480 has finished for PR 23499 at commit a175cc1.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@HeartSaVioR HeartSaVioR mentioned this pull request Jan 21, 2019
Closed
@dongjoon-hyun
Copy link
Member

dongjoon-hyun commented Jan 22, 2019

cc @wangyum since he is working on upgrading Hive dependency.

@SparkQA
Copy link

SparkQA commented Jan 23, 2019

Test build #101597 has finished for PR 23499 at commit d74140d.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

vanzin
vanzin reviewed Jan 23, 2019
View reviewed changes
@SparkQA
Copy link

SparkQA commented Jan 24, 2019

Test build #101638 has finished for PR 23499 at commit ab41a9a.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jan 25, 2019

Test build #101673 has finished for PR 23499 at commit 083199b.

  • This patch fails MiMa tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jan 25, 2019

Test build #101676 has finished for PR 23499 at commit 7b88592.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@vanzin
Copy link
Contributor

vanzin commented Jan 25, 2019

Merging to master.

@asfgit asfgit closed this in 773efed Jan 25, 2019
jackylee-ch pushed a commit to jackylee-ch/spark that referenced this pull request Feb 18, 2019
@gaborgsomogyi
[SPARK-26254][CORE] Extract Hive + Kafka dependencies from Core.
858b5fa 
## What changes were proposed in this pull request?

There are ugly provided dependencies inside core for the following:
* Hive
* Kafka

In this PR I've extracted them out. This PR contains the following:
* Token providers are now loaded with service loader
* Hive token provider moved to hive project
* Kafka token provider extracted into a new project

## How was this patch tested?

Existing + newly added unit tests.
Additionally tested on cluster.

Closes apache#23499 from gaborgsomogyi/SPARK-26254.

Authored-by: Gabor Somogyi <[email protected]>
Signed-off-by: Marcelo Vanzin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@viirya viirya viirya left review comments

@srowen srowen srowen left review comments

@vanzin vanzin vanzin left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@gaborgsomogyi @SparkQA @gatorsmile @vanzin @srowen @felixcheung @dongjoon-hyun @viirya