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

[Pulsar SQL] Upgrade Presto version to 334 to support Java 11.0.14.1 #14953

Closed
wants to merge 1 commit into from
Closed

[Pulsar SQL] Upgrade Presto version to 334 to support Java 11.0.14.1 #14953

wants to merge 1 commit into from

Conversation

lhotari
Copy link
Member

@lhotari lhotari commented Mar 30, 2022

Fixes #14951

Motivation

  • Fix compatibility with Java 11.0.14.1
  • ensure that airlift versions aren't mixed

Modifications

  • upgrade to Presto version 334 since the incompatiblity with Java 11.0.14.1 is resolved in that version
  • enable integration tests for Pulsar SQL again
  • prevent mixed airlift library versions by using a BOM
  • remove duplication in setting presto and airlift versions

@lhotari lhotari added area/sql Pulsar SQL related features doc-not-needed Your PR changes do not impact docs area/ci labels Mar 30, 2022
michaeljmarshall
michaeljmarshall approved these changes Mar 30, 2022
View reviewed changes
pom.xml
@@ -156,7 +156,7 @@ flexible messaging model and an intuitive client API.</description>
<hdfs-offload-version3>3.3.1</hdfs-offload-version3>
<json-smart.version>2.4.7</json-smart.version>
<opensearch.version>1.2.4</opensearch.version>
<presto.version>332</presto.version>
<presto.version>334</presto.version>
Copy link
Member

Choose a reason for hiding this comment

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

It's somewhat unrelated to this PR, but these are some very old versions of Presto. 334 was released in May 2020. https://trino.io/docs/current/release/release-334.html

Copy link
Contributor

Choose a reason for hiding this comment

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

We should move to Trino. But actually the plannis to drop this stuff here and move to the Trino project

Copy link
Member Author

Choose a reason for hiding this comment

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

The reason to upgrade to 334 is to do the minimal change to fix the issue. There might be incompatibilities in newer versions which would have to be addressed.
Yes, we should switch to Trino, but it seems that the PR by @MarvinCai trinodb/trino#8020 hasn't proceeded.

Copy link
Member

Choose a reason for hiding this comment

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

Since this drops support for JDK8 the documentation needs to be changed as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

I posted on dev mailing list about this: https://lists.apache.org/thread/3yhz8rql4hwch09728b6myfdq0o37gqj

Copy link
Member Author

Choose a reason for hiding this comment

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

#14988 contains a workaround to support both JDK 8 and JDK 11. That is needed for the maintenance branches. For master branch we are going to lift the requirement for the server side components soon, as discussed in the community meeting.

eolivelli
eolivelli requested changes Mar 30, 2022
View reviewed changes
Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

This change will require switching to jdk11 om the server side.

We are currently still supporting jdk8

We need to make a decision on dev@

@lhotari lhotari removed the doc-not-needed Your PR changes do not impact docs label Mar 30, 2022
@lhotari lhotari marked this pull request as draft March 30, 2022 17:58
@lhotari lhotari added the doc-required Your PR changes impact docs and you will update later. label Mar 30, 2022
@lhotari
Copy link
Member Author

lhotari commented Mar 30, 2022

I created #14954 with the changes for aligning Airlift library versions by using a BOM.

@lhotari lhotari mentioned this pull request Apr 19, 2022
Closed
@github-actions
Copy link

github-actions bot commented May 2, 2022

The pr had no activity for 30 days, mark with Stale label.

@github-actions
Copy link

github-actions bot commented Jun 2, 2022

The pr had no activity for 30 days, mark with Stale label.

@lhotari lhotari closed this Jun 7, 2022
@gaoran10 gaoran10 mentioned this pull request Jun 22, 2022
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eolivelli eolivelli eolivelli requested changes

@michaeljmarshall michaeljmarshall michaeljmarshall approved these changes

@merlimat merlimat Awaiting requested review from merlimat

@dlg99 dlg99 Awaiting requested review from dlg99

@codelipenghui codelipenghui Awaiting requested review from codelipenghui

@MarvinCai MarvinCai Awaiting requested review from MarvinCai

@wolfstudy wolfstudy Awaiting requested review from wolfstudy

@dave2wave dave2wave Awaiting requested review from dave2wave

Assignees

@lhotari lhotari

Labels
area/ci area/sql Pulsar SQL related features doc-required Your PR changes impact docs and you will update later. lifecycle/stale Stale
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Pulsar SQL won't start with Java 11.0.14.1
4 participants
@lhotari @eolivelli @dave2wave @michaeljmarshall