You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/10/28 20:33:44 UTC
[GitHub] [pulsar-client-go] zzzming opened a new pull request, #876: set minPartitionsAutoDiscoveryInterval to prevent partition metadata lookup overwhelm brokers
zzzming opened a new pull request, #876:
URL: https://github.com/apache/pulsar-client-go/pull/876
set minPartitionsAutoDiscoveryInterval to prevent partition metadata lookup overwhelm brokers
### Motivation
This issue is described by this client application's PR https://github.com/signalfx/splunk-otel-collector/pull/2185
On a 2.7 broker, a very short interval of partition auto discovery, the producer can overwhelm the broker. We have observed very high CPU usage. In an extreme case, a broker can run 100% CPU even without any topic loaded. The broker trace stack looks like
```
org.apache.pulsar.broker.service.PulsarCommandSenderImpl.sendPartitionMetadataResponse(PulsarCommandSenderImpl.java:65)
at org.apache.pulsar.broker.service.ServerCnx.lambda$null$7(ServerCnx.java:455)
at org.apache.pulsar.broker.service.ServerCnx$$Lambda$666/0x000000084070a440.apply(Unknown Source)
at java.util.concurrent.CompletableFuture.uniHandle(java.base@11.0.13/CompletableFuture.java:930)
at java.util.concurrent.CompletableFuture.uniHandleStage(java.base@11.0.13/CompletableFuture.java:946)
at java.util.concurrent.CompletableFuture.handle(java.base@11.0.13/CompletableFuture.java:2266)
at org.apache.pulsar.broker.service.ServerCnx.lambda$handlePartitionMetadataRequest$8(ServerCnx.java:452)
at org.apache.pulsar.broker.service.ServerCnx$$Lambda$661/0x0000000840708040.apply(Unknown Source)
at java.util.concurrent.CompletableFuture.uniApplyNow(java.base@11.0.13/CompletableFuture.java:680)
at java.util.concurrent.CompletableFuture.uniApplyStage(java.base@11.0.13/CompletableFuture.java:658)
at java.util.concurrent.CompletableFuture.thenApply(java.base@11.0.13/CompletableFuture.java:2094)
at org.apache.pulsar.broker.service.ServerCnx.handlePartitionMetadataRequest(ServerCnx.java:449)
at org.apache.pulsar.common.protocol.PulsarDecoder.channelRead(PulsarDecoder.java:122)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
at io.netty.handler.flow.FlowControlHandler.dequeue(FlowControlHandler.java:200)
at io.netty.handler.flow.FlowControlHandler.channelRead(FlowControlHandler.java:162)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
at io.netty.handler.codec.ByteToMessageDecoder.fireChannelRead(ByteToMessageDecoder.java:324)
at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:296)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
at io.netty.handler.ssl.SslHandler.unwrap(SslHandler.java:1534)
at io.netty.handler.ssl.SslHandler.decodeNonJdkCompatible(SslHandler.java:1295)
at io.netty.handler.ssl.SslHandler.decode(SslHandler.java:1332)
```
### Modifications
Set one second as the floor value for the PartitionsAutoDiscoveryInterval. This will prevent a high frequent look up call from the client.
### Verifying this change
- [x] Make sure that the change passes the CI checks.
### 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): (yes / no)
- The public API: (no)
- The schema: (no)
- The default values of configurations: (no)
- The wire protocol: (no)
### Documentation
- Does this pull request introduce a new feature? ( no)
- If yes, how is the feature documented? (not applicable)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] zzzming commented on a diff in pull request #876: set minPartitionsAutoDiscoveryInterval to prevent partition metadata lookup overwhelm brokers
Posted by GitBox <gi...@apache.org>.
zzzming commented on code in PR #876:
URL: https://github.com/apache/pulsar-client-go/pull/876#discussion_r1009426096
##########
pulsar/producer_impl.go:
##########
@@ -90,7 +93,7 @@ func newProducer(client *client, options *ProducerOptions) (*producer, error) {
if options.BatchingMaxPublishDelay <= 0 {
options.BatchingMaxPublishDelay = defaultBatchingMaxPublishDelay
}
- if options.PartitionsAutoDiscoveryInterval <= 0 {
+ if options.PartitionsAutoDiscoveryInterval <= minPartitionAutoDiscoveryInterval {
Review Comment:
Returns an error if PartitionsAutoDiscoveryInterval is less than 1 second
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] pgier commented on pull request #876: set minPartitionsAutoDiscoveryInterval to prevent partition metadata lookup overwhelm brokers
Posted by GitBox <gi...@apache.org>.
pgier commented on PR #876:
URL: https://github.com/apache/pulsar-client-go/pull/876#issuecomment-1297471705
It might be nice in the future to add a field partitionsAutoDiscoveryIntervalSeconds and deprecate the current one. Then the time unit is more clear to the user.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] lhotari commented on a diff in pull request #876: set minPartitionsAutoDiscoveryInterval to prevent partition metadata lookup overwhelm brokers
Posted by GitBox <gi...@apache.org>.
lhotari commented on code in PR #876:
URL: https://github.com/apache/pulsar-client-go/pull/876#discussion_r1008639592
##########
pulsar/producer_impl.go:
##########
@@ -90,7 +93,7 @@ func newProducer(client *client, options *ProducerOptions) (*producer, error) {
if options.BatchingMaxPublishDelay <= 0 {
options.BatchingMaxPublishDelay = defaultBatchingMaxPublishDelay
}
- if options.PartitionsAutoDiscoveryInterval <= 0 {
+ if options.PartitionsAutoDiscoveryInterval <= minPartitionAutoDiscoveryInterval {
Review Comment:
Wouldn't this be a problem if someone has set the value to <= 1 second? There's no error message and the default 1 minute gets used. I would expect at least a warning message. The case of 0 is different since in that case, the default value is expected.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-client-go] shibd commented on pull request #876: set minPartitionsAutoDiscoveryInterval to prevent partition metadata lookup overwhelm brokers
Posted by "shibd (via GitHub)" <gi...@apache.org>.
shibd commented on PR #876:
URL: https://github.com/apache/pulsar-client-go/pull/876#issuecomment-1423558551
> It might be nice in the future to add a field partitionsAutoDiscoveryIntervalSeconds and deprecate the current one. Then the time unit is more clear to the user.
+1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org