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

[Issue-11270] Change ContextClassLoader to NarClassLoader in ProtocolHandler #11276

Merged
merged 1 commit into from Jul 12, 2021

Conversation

Jason918
Copy link
Contributor

@Jason918 Jason918 commented Jul 10, 2021

Fixes #11270

Motivation

See #11270

Modifications

Change context class loader through Thread.currentThread().setContextClassLoader(classLoader) before every protocol handler callback, and change it back to original class loader afterwards.

Verifying this change

  • Make sure that the change passes the CI checks.

This change added tests and can be verified as follows:

see new test case in org.apache.pulsar.broker.protocol.ProtocolHandlerWithClassLoaderTest#testClassLoaderSwitcher.

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: (no)
  • The wire protocol: (no)
  • The rest endpoints: (no)
  • The admin cli options: (no)
  • Anything that affects deployment: (no)

Documentation

  • Does this pull request introduce a new feature? (no)
  • If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)
  • If a feature is not applicable for documentation, explain why?
  • If a feature is not documented yet in this PR, please create a followup issue for adding the documentation

@Anonymitaet
Copy link
Member

@Jason918
thanks for your contribution. For this PR, do we need to update docs?

@Jason918
Copy link
Contributor Author

/pulsarbot run-failure-checks

@Jason918
Copy link
Contributor Author

thanks for your contribution. For this PR, do we need to update docs?

@Anonymitaet Nop, I think it's transparent to the user and developer. Just fixing some conner issue.

@codelipenghui codelipenghui added the doc-not-needed Your PR changes do not impact docs label Jul 12, 2021
@codelipenghui codelipenghui added this to the 2.9.0 milestone Jul 12, 2021
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.

LGTM

tagging @merlimat @jerrypeng as we are working on the "classpath" problem for Pulsar IO

@eolivelli eolivelli merged commit 6887005 into apache:master Jul 12, 2021
codelipenghui pushed a commit that referenced this pull request Jul 14, 2021
Co-authored-by: Jiang Haiting <jianghaiting@didichuxing.com>
(cherry picked from commit 6887005)
@codelipenghui codelipenghui added the cherry-picked/branch-2.8 Archived: 2.8 is end of life label Jul 14, 2021
@Jason918 Jason918 deleted the issue-11270 branch July 26, 2021 02:54
bharanic-dev pushed a commit to bharanic-dev/pulsar that referenced this pull request Mar 18, 2022
…e#11276)

Co-authored-by: Jiang Haiting <jianghaiting@didichuxing.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/broker cherry-picked/branch-2.8 Archived: 2.8 is end of life doc-not-needed Your PR changes do not impact docs release/2.8.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use NarClassLoader in protocol hander callbacks.
5 participants