Skip to content

Conversation

@cxzl25
Copy link
Contributor

@cxzl25 cxzl25 commented Feb 8, 2023

Change Logs

Add guava dependency to Spark and MR bundle

Impact

Configure guava relocation in Spark and MR bundle pom.xml, but there is no guava dependency, resulting in failure of guava-related class loading.

[HUDI-4482] remove guava and use caffeine instead for cache (#6240)

Risk level (write none, low medium or high below)

Documentation Update

Contributor's checklist

  • Read through contributor's guide
  • Change Logs and Impact were stated clearly
  • Adequate tests were added if applicable
  • CI passed

@cxzl25
Copy link
Contributor Author

cxzl25 commented Feb 8, 2023

@pan3793 first found this problem.

cc @KnightChess @xushiyan

Copy link
Contributor

@alexeykudinkin alexeykudinkin left a comment

Choose a reason for hiding this comment

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

Thanks for contributing @KnightChess!

I don't think this is the right solution though -- bundling guava severely exposes Hudi to potential conflicts when running environment in turn provides Guava as well

@alexeykudinkin alexeykudinkin added the priority:blocker Production down; release blocker label Feb 8, 2023
@alexeykudinkin alexeykudinkin self-assigned this Feb 8, 2023
Copy link
Contributor

@yihua yihua left a comment

Choose a reason for hiding this comment

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

We can land this one as this brings back Guava dependencies as before such as 0.12.0 release. The shading and other issues should be tackled separately.

@alexeykudinkin
Copy link
Contributor

After some discussions we agreed on following

  • This PR will be landed to unblock 0.13
  • On master we revert back to remove guava as well as any excessive shading of com.google.common classes

@hudi-bot
Copy link
Collaborator

hudi-bot commented Feb 8, 2023

CI report:

Bot commands @hudi-bot supports the following commands:
  • @hudi-bot run azure re-run the last Azure build

@alexeykudinkin
Copy link
Contributor

#7900 is addressing this properly by removing unnecessary relocations

@bvaradar
Copy link
Contributor

@yihua @alexeykudinkin : Should this be closed since #7900 is landed ?

@alexeykudinkin
Copy link
Contributor

@bvaradar yes, we can close this one

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

Labels

priority:blocker Production down; release blocker

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

6 participants