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

[pulsar-shell] Load custom command NARs only once #20061

Closed
2 tasks done
aymkhalil opened this issue Apr 10, 2023 · 0 comments · Fixed by #20064
Closed
2 tasks done

[pulsar-shell] Load custom command NARs only once #20061

aymkhalil opened this issue Apr 10, 2023 · 0 comments · Fixed by #20064
Labels
type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages

Comments

@aymkhalil
Copy link
Contributor

Search before asking

  • I searched in the issues and found nothing similar.

Motivation

Pulsar-shell would attempt to load the NAR files every time a new custom/extention command is invoked (see PIP-201 for context regarding Pulsar Admin CLI Extensions).

This works fine with caveats:

  • Multiple NAR loading seems redundant. It is fair to expect the user to restart the shell session if the underlying NAR is updated.
  • It has some interesting consequences when the the extensions command has a runtime dependency on a class that is available on the parent class loader.

For example, I ran into a scenario where the extension/custom command has a dependency on netty-common - the command would load on the NAR class loader, but the netty dependency will load on the parent class loader (the admin's AppClassLoader). Between shell commands invocations, the classes in the NAR class loader are recycled, but the netty classes are not. This lead to any "static" state to appear as "leaking" when the CLI is invoked for the second time.

Solution

Implementing "singleton" custom command bootstrapping (the NAR class loader and other necessary initialization) would do, at least in theory.

Alternatives

Switch the order by which class loader work (child then parent) although this seems unnecessarily complex.

Anything else?

No response

Are you willing to submit a PR?

  • I'm willing to submit a PR!
@aymkhalil aymkhalil added the type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages label Apr 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant