-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[fix][fn]make sure the classloader for ContextImpl is functionClassLoader
in different runtimes
#22501
[fix][fn]make sure the classloader for ContextImpl is functionClassLoader
in different runtimes
#22501
Conversation
# Conflicts: # pulsar-functions/instance/src/main/java/org/apache/pulsar/functions/instance/JavaInstanceRunnable.java
@freeznet A question about the description:
|
The reason for "may" is that I am not sure why there is no related issue mentioned before. But from my testing with function-mesh, it always occurred, and we cannot observe the issue if we fallback to the previous version prior #20115. |
I'm not sure if @freeznet he has this issue every time on Kubernetes, but I have encountered it when using the extension SDK to connect to Redis cluster. I am currently unable to use functions on Kubernetes to complete my requirements.
|
...ctions/instance/src/main/java/org/apache/pulsar/functions/instance/JavaInstanceRunnable.java
Outdated
Show resolved
Hide resolved
@Awsmsniper this might be a different issue. please report it separately. |
@lhotari I have thought about it for a moment, and I believe the problem is caused by the same reason. Do I need another report? |
@Awsmsniper it should be another issue, could you please create a new issue first? Also, https://hub.docker.com/layers/freeznet/pulsar-all/3.0.5-SNAPSHOT-1c46877/images/sha256-d881a766d57b77ec4b279688b96b714f34eb270496b052eb69362db5c62d8bd3?context=repo is the docker image I built from this PR based on branch-3.0, you may check if this fix solves your issue as well. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approve based our internal tests have been passed.
I did not use the version you provided. After using version 3.0.4, I found that it can be used normally. Thank you. |
@lhotari could you please help to review this PR when you have time, thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…oader` in different runtimes (#22501)
…oader` in different runtimes (apache#22501) (cherry picked from commit d067efc) (cherry picked from commit d23c77b)
…oader` in different runtimes (apache#22501) (cherry picked from commit d067efc) (cherry picked from commit d23c77b)
Fixes #xyz
Main Issue: #xyz
PIP: #xyz
Motivation
#20115 fixes the classloader in TopicSchema, but for Kubernetes Runtime, the classloader may be different from the other runtimes. Which caused the SerDe to fail to init in Kubernetes Runtime.
Modifications
functionClassLoader
when init the contextVerifying this change
(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:)
Does this pull request potentially affect one of the following parts:
If the box was checked, please highlight the changes
Documentation
doc
doc-required
doc-not-needed
doc-complete
Matching PR in forked repository
PR in forked repository: freeznet#11