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/05/25 02:48:17 UTC
[GitHub] [pulsar] AnonHxy opened a new pull request, #15766: [fix][broker]Sync topicPublishRateLimiter update
AnonHxy opened a new pull request, #15766:
URL: https://github.com/apache/pulsar/pull/15766
### Motivation
* Synchronized `topicPublishRateLimiter` update action to prevent resource leakages:
* Thread1: `topicPublishRateLimiter ` is null, and and it will not invoke `close` in line:1216. Then it stop before line:1219
* Thread2: set `topicPublishRateLimiter` a new value, for example, `limiter2`. Then thread2 stop.
* Thread1: set `topicPublishRateLimiter ` as `PublishRateLimiter.DISABLED_RATE_LIMITER` at line:1219
* `limter2` will never be accessable, and may cause resource leakages.
https://github.com/apache/pulsar/blob/eee038870db95e009abde32e89b9d736a9e9cd1c/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/AbstractTopic.java#L1213-L1222
### Modifications
Sync `updatePublishDispatcher()'
### Verifying this change
- [x] Make sure that the change passes the CI checks.
### Documentation
- [x] `no-need-doc`
(Please explain why)
--
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] AnonHxy commented on pull request #15766: [fix][broker]Sync topicPublishRateLimiter update
Posted by GitBox <gi...@apache.org>.
AnonHxy commented on PR #15766:
URL: https://github.com/apache/pulsar/pull/15766#issuecomment-1137104228
https://github.com/apache/pulsar/pull/15599 solve the same problem , so close this PR
--
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] AnonHxy closed pull request #15766: [fix][broker]Sync topicPublishRateLimiter update
Posted by GitBox <gi...@apache.org>.
AnonHxy closed pull request #15766: [fix][broker]Sync topicPublishRateLimiter update
URL: https://github.com/apache/pulsar/pull/15766
--
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] Shawyeok commented on pull request #15766: [fix][broker]Sync topicPublishRateLimiter update
Posted by GitBox <gi...@apache.org>.
Shawyeok commented on PR #15766:
URL: https://github.com/apache/pulsar/pull/15766#issuecomment-1136906081
Ah, you are right, I missed preciseTopicPublishRateLimitingEnable=true branch which will create a `PrecisPublishLimiter`.
After `org.apache.pulsar.common.util.RateLimiter#createTask` was called, it is reachable from GC Roots.
--
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] Shawyeok commented on pull request #15766: [fix][broker]Sync topicPublishRateLimiter update
Posted by GitBox <gi...@apache.org>.
Shawyeok commented on PR #15766:
URL: https://github.com/apache/pulsar/pull/15766#issuecomment-1136718087
> Thread1: topicPublishRateLimiter is null, and and it will not invoke close in line:1216. Then it stop before line:1219
Thread2: set topicPublishRateLimiter a new value, for example, limiter2. Then thread2 stop.
Thread1: set topicPublishRateLimiter as PublishRateLimiter.DISABLED_RATE_LIMITER at line:1219
limter2 will never be accessable, and may cause resource leakages.
In scenario above, `limiter2` could be garbage collected, correct me if I miss something.
--
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] AnonHxy commented on pull request #15766: [fix][broker]Sync topicPublishRateLimiter update
Posted by GitBox <gi...@apache.org>.
AnonHxy commented on PR #15766:
URL: https://github.com/apache/pulsar/pull/15766#issuecomment-1136805735
>
I don't think so. In `close` method, it will close the `RateLimiter`, and the `RalteLimter#close` may shutdown executorService or cancel tasks to release some resources. @Shawyeok
--
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