-
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
[improve][java-client] Support passing existing scheduled executor providers to the client #16334
[improve][java-client] Support passing existing scheduled executor providers to the client #16334
Conversation
…oviders to the client ### Motivation apache#16236 introduced a new scheduled executor but does not support passing the existing scheduled executor providers like apache#12037.
@@ -196,8 +197,8 @@ private PulsarClientImpl(ClientConfigurationData conf, EventLoopGroup eventLoopG | |||
new ExecutorProvider(conf.getNumListenerThreads(), "pulsar-external-listener"); | |||
this.internalExecutorProvider = internalExecutorProvider != null ? internalExecutorProvider : | |||
new ExecutorProvider(conf.getNumIoThreads(), "pulsar-client-internal"); | |||
this.scheduledExecutorProvider = new ScheduledExecutorProvider(conf.getNumIoThreads(), | |||
"pulsar-client-scheduled"); | |||
this.scheduledExecutorProvider = scheduledExecutorProvider != null ? scheduledExecutorProvider : |
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.
Shouldn’t we consider whether it was passed in or created internally, at the moment of closing it?
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.
@merlimat Fixed.
### Motivation apache#16334 supports passing scheduled executor to the client. Use a shared scheduled executor for the broker clients which used by replicator and system topics.
…oviders to the client (apache#16334) * [improve][java-client] Support passing existing scheduled executor providers to the client ### Motivation apache#16236 introduced a new scheduled executor but does not support passing the existing scheduled executor providers like apache#12037. * Apply comment.
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, thanks @codelipenghui
…oviders to the client (#16334) * [improve][java-client] Support passing existing scheduled executor providers to the client ### Motivation #16236 introduced a new scheduled executor but does not support passing the existing scheduled executor providers like #12037. * Apply comment. (cherry picked from commit f159f62)
…oviders to the client (apache#16334) * [improve][java-client] Support passing existing scheduled executor providers to the client ### Motivation apache#16236 introduced a new scheduled executor but does not support passing the existing scheduled executor providers like apache#12037. * Apply comment. (cherry picked from commit f159f62) (cherry picked from commit 4562d25)
…oviders to the client (apache#16334) * [improve][java-client] Support passing existing scheduled executor providers to the client ### Motivation apache#16236 introduced a new scheduled executor but does not support passing the existing scheduled executor providers like apache#12037. * Apply comment.
…oviders to the client (#16334) * [improve][java-client] Support passing existing scheduled executor providers to the client ### Motivation #16236 introduced a new scheduled executor but does not support passing the existing scheduled executor providers like #12037. * Apply comment. (cherry picked from commit f159f62)
…oviders to the client (#16334) * [improve][java-client] Support passing existing scheduled executor providers to the client ### Motivation #16236 introduced a new scheduled executor but does not support passing the existing scheduled executor providers like #12037. * Apply comment. (cherry picked from commit f159f62)
Motivation
#16236 introduced a new scheduled executor but does not support passing the existing scheduled executor providers like #12037.
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)
doc-not-needed
(Please explain why)
doc
(Your PR contains doc changes)
doc-complete
(Docs have been already added)