Skip to content

PARQUET-2173. Fix parquet build against hadoop 3.3.3+ - #985

Merged
gszadovszky merged 1 commit into
apache:masterfrom
steveloughran:PARQUET-2173-hadoop-3.3.4-build-support
Feb 2, 2023
Merged

PARQUET-2173. Fix parquet build against hadoop 3.3.3+#985
gszadovszky merged 1 commit into
apache:masterfrom
steveloughran:PARQUET-2173-hadoop-3.3.4-build-support

Conversation

@steveloughran

Copy link
Copy Markdown
Contributor

Hadoop 3.3.3 moved to reload4j for logging to stop
shipping a version of log4j with known (albeit unused)
CVEs.

This bypasses the existing exclusion code used to
keep hadoop's SLF4J dependency off the classpaths,
and by adding a new jar, breaks parquet-cli build.

Make sure you have checked all steps below.

Jira

Tests

  • My PR adds the following unit tests OR does not need testing for this extremely good reason:

The testing is regression testing "does the build work?", "does a test run complete without SLF4J warnings of duplicates?". done manually with -Dhadoop.version=3.3.4

Commits

  • My commits all reference Jira issues in their subject lines. In addition, my commits follow the guidelines from "How to write a good git commit message":
    1. Subject is separated from body by a blank line
    2. Subject is limited to 50 characters (not including Jira issue reference)
    3. Subject does not end with a period
    4. Subject uses the imperative mood ("add", not "adding")
    5. Body wraps at 72 characters
    6. Body explains "what" and "why", not "how"

Documentation

  • In case of new functionality, my PR adds documentation that describes how to use it.
    • All the public functions and the classes in the PR contain Javadoc that explain what it does

@steveloughran

Copy link
Copy Markdown
Contributor Author

i've also built against the next release of hadoop, and of 3.4.0-SNAPSHOT.

the parquet build fails there as jackson 1 is purged from the hadoop classpath, breaking the japicmp plugin.

Execution default of goal com.github.siom79.japicmp:japicmp-maven-plugin:0.14.2:cmp failed: Could not load 'org.codehaus.jackson.type.TypeReference

Comment thread pom.xml
<exclusion>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-log4j12</artifactId>
<artifactId>*</artifactId>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

'*' might be too broad

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

it means that

  1. classic log4j is excluded
  2. slf4j-reload4j is excluded
  3. when the hadoop move to log4j2 is finally shipped, its slf4j bindings will be excluded too.

exclusions 1 and 2 could be done explicitly, but #3 is unclear until something ships. (hadoop trunk/3.4 is on log4j2, but the move isn't complete)

1.Exclude reload4j dependencies from hadoop modules so
avoiding parquet cli convergence issues.
2.Add reload4j as a dependency to reject in compile scope.
3.Expand slf4j artifact exclusion in hadoop-* dependencies

These changes are compatible with hadoop < 3.3.3; they simply
ensure that on later versions, the reload4j dependencies
don't get picked up.
@steveloughran
steveloughran force-pushed the PARQUET-2173-hadoop-3.3.4-build-support branch from d33a983 to 9f31cb3 Compare December 2, 2022 11:45
@shangxinli

Copy link
Copy Markdown
Contributor

cc @ggershinsky @wgtmac let me know if you have concern to merge.

@wgtmac

wgtmac commented Dec 6, 2022

Copy link
Copy Markdown
Member

cc @ggershinsky @wgtmac let me know if you have concern to merge.

Thanks for pinging me! I don't have any concern for now.

@steveloughran

Copy link
Copy Markdown
Contributor Author

be good to get this in. FWIW i've been trying to build lots of things with the current smoke-build of a hadoop 3.3.5 RC. There's some aspects of maven playing up and I can't get parquet to collect the JARs from the asf staging repo, even with a profile in ~/.m2/settings.xml for it. I will probably have to add an explicit profile for that in the parquet build.

@ggershinsky

Copy link
Copy Markdown
Contributor

cc @ggershinsky @wgtmac let me know if you have concern to merge.

Thanks for pinging me! I don't have any concern for now.

Same here

@steveloughran

Copy link
Copy Markdown
Contributor Author

any plans to merge now?

@wgtmac

wgtmac commented Feb 1, 2023

Copy link
Copy Markdown
Member

It looks good to me but I don't have the privilege to merge.

May I request your help? @ggershinsky @shangxinli @gszadovszky

@gszadovszky
gszadovszky merged commit fd1326a into apache:master Feb 2, 2023
@steveloughran

Copy link
Copy Markdown
Contributor Author

thanks; closed the jira

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.

5 participants