[patch][pulsar-sql] Add Java version trim agent for presto 332 #15236

gaoran10 · 2022-04-20T11:08:35Z

Motivation

The presto 332 couldn't parse Java version like this 11.0.14.1, so add a Java version trim agent to walk around the problem.

This is a temporary patch, after the presto upgrade to 332+, we could remove this.

Modifications

Add a Java version trim agent to format the system property java.version for the Pulsar SQL plugin.

Verifying this change

Make sure that the change passes the CI checks.

(Please pick either of the following options)

This change is a trivial rework / code cleanup without any test coverage.

(or)

This change is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(example:)

Added integration tests for end-to-end deployment with large payloads (10MB)
Extended integration test for recovery after broker failure

Does this pull request potentially affect one of the following parts:

If yes was chosen, please highlight the changes

Dependencies (does it add or upgrade a dependency): (no)
The public API: (no)
The schema: (no)
The default values of configurations: (yes)
The wire protocol: (no)
The rest endpoints: (no)
The admin cli options: (no)
Anything that affects deployment: (no)

Documentation

Check the box below or label this PR directly.

Need to update docs?

doc-required
(Your PR needs to update docs and you will update later)
no-need-doc
(Please explain why)
doc
(Your PR contains doc changes)
doc-added
(Docs have been already added)

lhotari

Good work! A few minor changes

.../java-version-trim-agent/src/main/java/org/apache/pulsar/sql/agent/TrimJavaVersionAgent.java

pulsar-sql/presto-distribution/src/assembly/assembly.xml

pulsar-sql/java-version-trim-agent/pom.xml

lhotari · 2022-04-21T06:29:01Z

@gaoran10 Please reply to my review comments.

gaoran10 · 2022-04-21T09:36:58Z

@lhotari Thanks for your suggestions, and address comments. PTAL

lhotari · 2022-04-21T10:04:37Z

...integration/src/test/java/org/apache/pulsar/tests/integration/suites/PulsarSQLTestSuite.java

-        envMap.put("managedLedgerMaxEntriesPerLedger", String.valueOf(ENTRIES_PER_LEDGER));
-        envMap.put("managedLedgerMinLedgerRolloverTimeMinutes", "0");


these settings might be needed for other tests, not just PulsarSQL offloading tests.

These configurations are only needed by the test TestPrestoQueryTieredStorage.java, they are used to control offload operation. The TestBasicPresto.java don't need them.

Please take care of that in a separate PR

lhotari

Why are you making changes to TestPrestoQueryTieredStorage.java and PulsarSQLTestSuite.java as part of this PR? I think those changes should be reverted.

…undError: org/apache/pulsar/shade/javax/annotation/Priority. ``` Caused by: java.lang.NoClassDefFoundError: org/apache/pulsar/shade/javax/annotation/Priority at org.apache.pulsar.shade.org.glassfish.jersey.model.internal.CommonConfig$FeatureRegistration.priority(CommonCon {} at org.apache.pulsar.shade.org.glassfish.jersey.model.internal.CommonConfig$FeatureRegistration.<init>(CommonConfig.java:119) at org.apache.pulsar.shade.org.glassfish.jersey.model.internal.CommonConfig$FeatureRegistration.<init>(CommonConfig.java:107) at org.apache.pulsar.shade.org.glassfish.jersey.model.internal.CommonConfig.processFeatureRegistration(CommonConfig.java:511) at org.apache.pulsar.shade.org.glassfish.jersey.model.internal.CommonConfig.register(CommonConfig.java:418) at org.apache.pulsar.shade.org.glassfish.jersey.client.ClientConfig$State.register(ClientConfig.java:216) at org.apache.pulsar.shade.org.glassfish.jersey.client.ClientConfig.register(ClientConfig.java:606) at org.apache.pulsar.client.admin.internal.PulsarAdminImpl.<init>(PulsarAdminImpl.java:179) at org.apache.pulsar.client.admin.internal.PulsarAdminBuilderImpl.build(PulsarAdminBuilderImpl.java:47) at org.apache.pulsar.sql.presto.PulsarConnectorConfig.getPulsarAdmin(PulsarConnectorConfig.java:474) ... 51 more {} Caused by: java.lang.ClassNotFoundException: org.apache.pulsar.shade.javax.annotation.Priority at java.base/java.net.URLClassLoader.findClass(URLClassLoader.java:445) at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:587) at io.prestosql.server.PluginClassLoader.loadClass(PluginClassLoader.java:89) at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:520) ... 61 more ```

gaoran10 · 2022-04-22T14:39:05Z

@codelipenghui @lhotari All comments are resolved. PTAL, thanks

lhotari

Good work!

…e#15236) Fix apache#14951 The presto 332 couldn't parse Java version like this `11.0.14.1`, so add a Java version trim agent to walk around the problem. This is a temporary patch, after the presto upgrade to 332+, we could remove this. Add a Java version trim agent to format the system property `java.version` for the Pulsar SQL plugin. (cherry picked from commit a1aa18f)

codelipenghui · 2022-04-28T13:16:58Z

@gaoran10 I'm not able to cherry-pick this one to branch-2.10, could you please help with this or push a PR to branch-2.10 directly.

codelipenghui · 2022-04-28T13:18:26Z

421e7c7 for cherry-picking to branch-2.9

…e#15236) Fix apache#14951 The presto 332 couldn't parse Java version like this `11.0.14.1`, so add a Java version trim agent to walk around the problem. This is a temporary patch, after the presto upgrade to 332+, we could remove this. Add a Java version trim agent to format the system property `java.version` for the Pulsar SQL plugin. (cherry picked from commit a1aa18f)

…#15373) * [patch][pulsar-sql] Add Java version trim agent for presto 332 (#15236) Fix #14951 The presto 332 couldn't parse Java version like this `11.0.14.1`, so add a Java version trim agent to walk around the problem. This is a temporary patch, after the presto upgrade to 332+, we could remove this. Add a Java version trim agent to format the system property `java.version` for the Pulsar SQL plugin. (cherry picked from commit a1aa18f) * fix presto License check and version

…e#15236) Fix apache#14951 The presto 332 couldn't parse Java version like this `11.0.14.1`, so add a Java version trim agent to walk around the problem. This is a temporary patch, after the presto upgrade to 332+, we could remove this. Add a Java version trim agent to format the system property `java.version` for the Pulsar SQL plugin. (cherry picked from commit a1aa18f)

codelipenghui · 2022-05-20T12:48:00Z

#15371 cherrypick to branch-2.10

github-actions bot assigned gaoran10 Apr 20, 2022

github-actions bot added the doc-not-needed Your PR changes do not impact docs label Apr 20, 2022

gaoran10 requested review from lhotari, 315157973, BewareMyPower, codelipenghui, congbobo184, eolivelli, hangc0276 and Technoboy- April 20, 2022 11:09

gaoran10 added the area/sql Pulsar SQL related features label Apr 20, 2022

lhotari requested changes Apr 20, 2022

View reviewed changes

codelipenghui reviewed Apr 20, 2022

View reviewed changes

pulsar-sql/java-version-trim-agent/pom.xml Outdated Show resolved Hide resolved

lhotari reviewed Apr 21, 2022

View reviewed changes

pulsar-sql/java-version-trim-agent/pom.xml Outdated Show resolved Hide resolved

gaoran10 added 5 commits April 21, 2022 14:38

Add java version trim agent for presto 332.

9056737

enable sql integration test

a5c0229

fix test

2e2e6ba

fix test

64437ae

address comment

86f12c6

gaoran10 force-pushed the java-version-trim-agent-presto332 branch from 80e22d4 to 86f12c6 Compare April 21, 2022 09:34

lhotari reviewed Apr 21, 2022

View reviewed changes

lhotari requested changes Apr 21, 2022

View reviewed changes

gaoran10 added 2 commits April 22, 2022 01:04

add java agent config in the test jvm.config

ab39c02

lhotari approved these changes Apr 22, 2022

View reviewed changes

codelipenghui approved these changes Apr 24, 2022

View reviewed changes

codelipenghui merged commit a1aa18f into apache:master Apr 24, 2022

codelipenghui added release/2.9.3 release/2.10.1 labels Apr 24, 2022

codelipenghui added this to the 2.11.0 milestone Apr 24, 2022

gaoran10 deleted the java-version-trim-agent-presto332 branch April 24, 2022 06:10

codelipenghui added the cherry-picked/branch-2.9 Archived: 2.9 is end of life label Apr 26, 2022

gaoran10 restored the java-version-trim-agent-presto332 branch May 10, 2022 04:12

codelipenghui added the cherry-picked/branch-2.10 label May 20, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[patch][pulsar-sql] Add Java version trim agent for presto 332 #15236

[patch][pulsar-sql] Add Java version trim agent for presto 332 #15236

gaoran10 commented Apr 20, 2022 •

edited

Loading

lhotari left a comment

lhotari commented Apr 21, 2022

gaoran10 commented Apr 21, 2022

lhotari Apr 21, 2022

gaoran10 Apr 21, 2022

lhotari Apr 21, 2022

gaoran10 Apr 22, 2022

lhotari left a comment

gaoran10 commented Apr 22, 2022

lhotari left a comment

codelipenghui commented Apr 28, 2022

codelipenghui commented Apr 28, 2022

codelipenghui commented May 20, 2022

		envMap.put("managedLedgerMaxEntriesPerLedger", String.valueOf(ENTRIES_PER_LEDGER));
		envMap.put("managedLedgerMinLedgerRolloverTimeMinutes", "0");

[patch][pulsar-sql] Add Java version trim agent for presto 332 #15236

[patch][pulsar-sql] Add Java version trim agent for presto 332 #15236

Conversation

gaoran10 commented Apr 20, 2022 • edited Loading

Motivation

Modifications

Verifying this change

Does this pull request potentially affect one of the following parts:

Documentation

lhotari left a comment

Choose a reason for hiding this comment

lhotari commented Apr 21, 2022

gaoran10 commented Apr 21, 2022

lhotari Apr 21, 2022

Choose a reason for hiding this comment

gaoran10 Apr 21, 2022

Choose a reason for hiding this comment

lhotari Apr 21, 2022

Choose a reason for hiding this comment

gaoran10 Apr 22, 2022

Choose a reason for hiding this comment

lhotari left a comment

Choose a reason for hiding this comment

gaoran10 commented Apr 22, 2022

lhotari left a comment

Choose a reason for hiding this comment

codelipenghui commented Apr 28, 2022

codelipenghui commented Apr 28, 2022

codelipenghui commented May 20, 2022

gaoran10 commented Apr 20, 2022 •

edited

Loading