Skip to content
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

DRILL-8436: Upgrade Hadoop 3.2.4 → 3.3.6 #2821

Merged
merged 6 commits into from
Aug 25, 2023
Merged

DRILL-8436: Upgrade Hadoop 3.2.4 → 3.3.6 #2821

merged 6 commits into from
Aug 25, 2023

Conversation

jnturton
Copy link
Contributor

@jnturton jnturton commented Aug 11, 2023 •
edited
Loading

DRILL-8436: Upgrade Hadoop 3.2.4 → 3.3.6

Description

Hadoop is upgraded to 3.3.6.

Documentation

N/A

Testing

Existing unit tests, manual testing of Drill HTTP services. Manual testing Drill JDBC driver.

Rebased onto #2823.

@jnturton jnturton force-pushed the 8436-hadoop-3.3 branch 3 times, most recently from fff8e71 to 131ea54 Compare August 16, 2023 11:45
@jnturton jnturton self-assigned this Aug 16, 2023
@jnturton jnturton marked this pull request as ready for review August 16, 2023 11:49
@jnturton jnturton changed the title DRILL-8436: Upgrade Hadoop 3.2.4 -> 3.3.6 DRILL-8436: Upgrade Hadoop 3.2.4 → 3.3.6 Aug 16, 2023
@jnturton
Copy link
Contributor Author

So, this seems to work but not in JDK 8 😒

@jnturton jnturton mentioned this pull request Aug 16, 2023
Merged
pjfanning
pjfanning reviewed Aug 16, 2023
View reviewed changes
exec/jdbc-all/pom.xml Outdated
<artifactId>jersey-server</artifactId>
</exclusion>
<exclusion>
<groupId>com.sun.jersey</groupId>
Copy link
Contributor

Choose a reason for hiding this comment

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

hadoop 3.3.6 uses my fork of jersey-json -- see https://mvnrepository.com/artifact/org.apache.hadoop/hadoop-common/3.3.6

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah! Thanks.

@pjfanning
Copy link
Contributor

pjfanning commented Aug 16, 2023 •
edited
Loading

So, this seems to work but not in JDK 8 😒

In JDK 8, it can't find io/netty/handler/codec/http/HttpRequest. Maybe, we need to add an explicit dependency on the io.netty:netty-codec-http jar

@jnturton jnturton requested a review from pjfanning August 17, 2023 09:10
@jnturton jnturton marked this pull request as draft August 17, 2023 13:57
@jnturton
Copy link
Contributor Author

I've just set this PR to Draft because I rediscovered a problem in the Drill JDBC driver. I'll paste a chat message I sent to @vvysotskyi a few months back below, to reveal the nature of the problem. I'm sure it's ultimately fixable, but I don't know of an elegant fix yet.

Hi Vova! I decided to try upgrading Drill's Hadoop libs to 3.3.5. Things are working but there is a problem in the Drill JDBC fat jar. There, the shade plugin relocates Hadoop to underneath oadd as usual but now there are class names present in the core-default.xml file in hadoop-common.jar which are not updated by the shade plugin. The result is that the JDBC driver is broken. While the shade plugin can update some kinds of text config files, it doesn't appear that it can update arbitrary XML config like core-default.xml. I thought of including our own manually updated copy of core-default.xml in exec/jdbc-all/src/resources and trying to make sure the shade plugin picks that one instead of the one in hadoop-common.jar. My only reservation is that introducing this copy creates a maintenance burden for the future so I thought to ask you if you have any ideas...

cgivre
cgivre approved these changes Aug 18, 2023
View reviewed changes
Copy link
Contributor

@cgivre cgivre left a comment

Choose a reason for hiding this comment

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

LGTM +1

@jnturton jnturton marked this pull request as ready for review August 22, 2023 13:59
@jnturton
Copy link
Contributor Author

I've got the JDBC driver working by bundling a core-site.xml file in it that handles the relocation of org.apache.hadoop to oadd.org.apache.hadoop.

cgivre
cgivre approved these changes Aug 23, 2023
View reviewed changes
Copy link
Contributor

@cgivre cgivre left a comment

Choose a reason for hiding this comment

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

LGTM +1. Thanks for this!

@cgivre
Copy link
Contributor

cgivre commented Aug 23, 2023

I thought I was reviewing the other PR for the library updates. Could we rebase this on master once that has been merged?

@jnturton
Copy link
Contributor Author

I thought I was reviewing the other PR for the library updates. Could we rebase this on master once that has been merged?

Done.

@jnturton jnturton force-pushed the 8436-hadoop-3.3 branch 2 times, most recently from 4286115 to b057ce2 Compare August 24, 2023 06:27
@jnturton jnturton requested a review from cgivre August 24, 2023 07:04
jnturton
jnturton commented Aug 25, 2023
View reviewed changes
contrib/storage-phoenix/pom.xml Outdated
@@ -29,9 +29,9 @@
<name>Drill : Contrib : Storage : Phoenix</name>

<properties>
<phoenix.version>5.1.2</phoenix.version>
<phoenix.version>5.1.3</phoenix.version>
<!-- Keep the 2.4.2 to reduce dependency conflict -->
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cgivre I need to reword this comment slightly and prune some commented sections from java-exec/pom.xml below, but are you happy with this PR at this stage?

jnturton
jnturton commented Aug 25, 2023
View reviewed changes
exec/jdbc-all/pom.xml
@@ -388,51 +396,52 @@
<include>*:*</include>
</includes>
<excludes>
<exclude>io.protostuff:*</exclude>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cgivre All the diff noise here comes about because I decided to sort this list alphabetically in the hopes that contributors will keep it sorted from here on, making checking what's in it that much easier. I can revert the sorting if preferred though.

jnturton
jnturton commented Aug 25, 2023
View reviewed changes
exec/jdbc-all/pom.xml
@@ -679,86 +688,85 @@
<filter>
<artifact>*:*</artifact>
<excludes>
<exclude>**/logback.xml</exclude>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Another list that's been sorted alphabetically in this PR.

cgivre
cgivre approved these changes Aug 25, 2023
View reviewed changes
Copy link
Contributor

@cgivre cgivre left a comment

Choose a reason for hiding this comment

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

LGTM +1

@jnturton jnturton merged commit 9813d15 into apache:master Aug 25, 2023
2 of 7 checks passed
@jnturton jnturton deleted the 8436-hadoop-3.3 branch August 25, 2023 12:43
cgivre pushed a commit to cgivre/drill that referenced this pull request Nov 2, 2023
@jnturton @cgivre
DRILL-8436: Upgrade Hadoop 3.2.4 → 3.3.6 (apache#2821)
cd40d41 
* Upgrade Hadoop to 3.3.6.
* Upgrade Apache Phoenix libs to 5.1.3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cgivre cgivre cgivre approved these changes

@vvysotskyi vvysotskyi Awaiting requested review from vvysotskyi

@vdiravka vdiravka Awaiting requested review from vdiravka

@pjfanning pjfanning Awaiting requested review from pjfanning

Assignees

@jnturton jnturton

Labels
dependencies
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jnturton @pjfanning @cgivre