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.

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

Improve boostrap log to expose the wrong classLoader #24533

Merged
merged 1 commit into from
Feb 18, 2025

Conversation

ericyuliu
Copy link
Contributor

@ericyuliu ericyuliu commented Feb 11, 2025

Summary:
Improve bootstrap error message for incorrect classloader to include what classloader was found.

@facebook-github-bot
Copy link
Collaborator

This pull request was exported from Phabricator. Differential Revision: D69416153

@facebook-github-bot
Copy link
Collaborator

This pull request was exported from Phabricator. Differential Revision: D69416153

@rschlussel
Copy link
Contributor

nit: capitalize the commit title and remove references to internal connectors.

@jaystarshot
Copy link
Member

Looks like you need a rebase to fix the maven checks

ericyuliu added a commit to ericyuliu/presto that referenced this pull request Feb 18, 2025
Summary:

Impulse class loader issue causing bootstrap exception

Prev
Caused by: java.lang.IllegalArgumentException: Expected com.facebook.presto.$gen.WeightedAverageStateSerializer_20250210_222951_7's classloader to be of type com.facebook.presto.bytecode.DynamicClassLoader, but is com.facebook.presto.bytecode.DynamicClassLoader

Differential Revision: D69416153
@facebook-github-bot
Copy link
Collaborator

This pull request was exported from Phabricator. Differential Revision: D69416153

@ericyuliu ericyuliu changed the title improve boostrap log to expose the wrong classLoader Improve boostrap log to expose the wrong classLoader Feb 18, 2025
@rschlussel
Copy link
Contributor

You still need to fix the commit message body. The current commit body includes references to an internal connector not in open source and isn't very comprehensible to someone without context. The body here doesn't actually need much because the change is pretty self explanatory. Something like the following should be sufficient: "Improve bootstrap error message for incorrect classloader to include what classloader was found."

Summary:

Improve bootstrap error message for incorrect classloader to include what classloader was found.

Differential Revision: D69416153
@facebook-github-bot
Copy link
Collaborator

This pull request was exported from Phabricator. Differential Revision: D69416153

@ericyuliu
Copy link
Contributor Author

You still need to fix the commit message body. The current commit body includes references to an internal connector not in open source and isn't very comprehensible to someone without context. The body here doesn't actually need much because the change is pretty self explanatory. Something like the following should be sufficient: "Improve bootstrap error message for incorrect classloader to include what classloader was found."

Fixed, thanks!

@rschlussel
Copy link
Contributor

Thanks!

@rschlussel rschlussel merged commit d7c0930 into prestodb:master Feb 18, 2025
55 checks passed
jp-sivaprasad pushed a commit to jp-sivaprasad/presto that referenced this pull request Mar 10, 2025
Summary:

Improve bootstrap error message for incorrect classloader to include what classloader was found.

Differential Revision: D69416153
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants