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 2020/05/28 05:09:39 UTC
[GitHub] [pulsar] aloyszhang opened a new pull request #7078: introduce precise topic publish rate limiting
aloyszhang opened a new pull request #7078:
URL: https://github.com/apache/pulsar/pull/7078
Fixes #6975
Master Issue: #6975
### Motivation
Enable precise topic publish rate limit on broker.
Now pulsar limits publish rate of topic by a period task runs every `topicPublisherThrottlingTickTimeMillis`. That means in the time `topicPublisherThrottlingTickTimeMillis`, the limit can't take effect.
So, we should provide a precise way to do this.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] aloyszhang removed a comment on pull request #7078: introduce precise topic publish rate limiting
Posted by GitBox <gi...@apache.org>.
aloyszhang removed a comment on pull request #7078:
URL: https://github.com/apache/pulsar/pull/7078#issuecomment-635187954
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui commented on pull request #7078: introduce precise topic publish rate limiting
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on pull request #7078:
URL: https://github.com/apache/pulsar/pull/7078#issuecomment-637234296
@aloyszhang nice contribution!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] aloyszhang commented on pull request #7078: introduce precise topic publish rate limiting
Posted by GitBox <gi...@apache.org>.
aloyszhang commented on pull request #7078:
URL: https://github.com/apache/pulsar/pull/7078#issuecomment-635187954
/pulsarbot run-failure-checks
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] aloyszhang commented on pull request #7078: introduce precise topic publish rate limiting
Posted by GitBox <gi...@apache.org>.
aloyszhang commented on pull request #7078:
URL: https://github.com/apache/pulsar/pull/7078#issuecomment-636688304
/pulsarbot run-failure-checks
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] aloyszhang commented on pull request #7078: introduce precise topic publish rate limiting
Posted by GitBox <gi...@apache.org>.
aloyszhang commented on pull request #7078:
URL: https://github.com/apache/pulsar/pull/7078#issuecomment-637232028
@codelipenghui I have add tests for preciseTopicPublishRateLimite, and all check passed. PTAL
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] aloyszhang commented on pull request #7078: introduce precise topic publish rate limiting
Posted by GitBox <gi...@apache.org>.
aloyszhang commented on pull request #7078:
URL: https://github.com/apache/pulsar/pull/7078#issuecomment-636631495
@codelipenghui Add tests, PTAL
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] rdhabalia commented on pull request #7078: introduce precise topic publish rate limiting
Posted by GitBox <gi...@apache.org>.
rdhabalia commented on pull request #7078:
URL: https://github.com/apache/pulsar/pull/7078#issuecomment-635741832
what if we reduce `topicPublisherThrottlingTickTimeMillis=1` to make it more precise? main motivation of `tickTime` to have tradeoff between preciseness and cpu overhead.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] aloyszhang commented on a change in pull request #7078: introduce precise topic publish rate limiting
Posted by GitBox <gi...@apache.org>.
aloyszhang commented on a change in pull request #7078:
URL: https://github.com/apache/pulsar/pull/7078#discussion_r432435725
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/PublishRateLimiter.java
##########
@@ -66,6 +69,83 @@
* @param clusterName
*/
void update(PublishRate maxPublishRate);
+
+ /**
+ * try to acquire permit
+ * @param numbers
+ * @param bytes
+ * */
+ boolean tryAcquire(int numbers, long bytes);
+}
+
+class PrecisPublishLimiter implements PublishRateLimiter {
+ protected volatile int publishMaxMessageRate = 0;
Review comment:
@merlimat There will be barely concurrent update of these fields, so I think volatile here is ok. Do you suggest to remove it ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] aloyszhang removed a comment on pull request #7078: introduce precise topic publish rate limiting
Posted by GitBox <gi...@apache.org>.
aloyszhang removed a comment on pull request #7078:
URL: https://github.com/apache/pulsar/pull/7078#issuecomment-636688304
/pulsarbot run-failure-checks
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui merged pull request #7078: introduce precise topic publish rate limiting
Posted by GitBox <gi...@apache.org>.
codelipenghui merged pull request #7078:
URL: https://github.com/apache/pulsar/pull/7078
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] aloyszhang removed a comment on pull request #7078: introduce precise topic publish rate limiting
Posted by GitBox <gi...@apache.org>.
aloyszhang removed a comment on pull request #7078:
URL: https://github.com/apache/pulsar/pull/7078#issuecomment-636631495
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] aloyszhang commented on pull request #7078: introduce precise topic publish rate limiting
Posted by GitBox <gi...@apache.org>.
aloyszhang commented on pull request #7078:
URL: https://github.com/apache/pulsar/pull/7078#issuecomment-635721485
I'll add some test.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] merlimat commented on a change in pull request #7078: introduce precise topic publish rate limiting
Posted by GitBox <gi...@apache.org>.
merlimat commented on a change in pull request #7078:
URL: https://github.com/apache/pulsar/pull/7078#discussion_r432263968
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/PublishRateLimiter.java
##########
@@ -66,6 +69,83 @@
* @param clusterName
*/
void update(PublishRate maxPublishRate);
+
+ /**
+ * try to acquire permit
+ * @param numbers
+ * @param bytes
+ * */
+ boolean tryAcquire(int numbers, long bytes);
+}
+
+class PrecisPublishLimiter implements PublishRateLimiter {
+ protected volatile int publishMaxMessageRate = 0;
Review comment:
I'd say in general to avoid volatile in all cases in which the exact semantic of read after write consistency is required. In this case we can probably just rely on dirty reads
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] aloyszhang commented on pull request #7078: introduce precise topic publish rate limiting
Posted by GitBox <gi...@apache.org>.
aloyszhang commented on pull request #7078:
URL: https://github.com/apache/pulsar/pull/7078#issuecomment-636779830
/pulsarbot run-failure-checks
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] aloyszhang commented on pull request #7078: introduce precise topic publish rate limiting
Posted by GitBox <gi...@apache.org>.
aloyszhang commented on pull request #7078:
URL: https://github.com/apache/pulsar/pull/7078#issuecomment-635754644
@rdhabalia Agree with you than `topicPublisherThrottlingTickTimeMillis=1` can provide a morr precise rate limit, but will cause more cpu overhead.
Anohter scenario, if publish rate is very high while publish-rate-limit is quite low, enven
with `topicPublisherThrottlingTickTimeMillis=1`, rate limit still lose effect.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] aloyszhang commented on pull request #7078: introduce precise topic publish rate limiting
Posted by GitBox <gi...@apache.org>.
aloyszhang commented on pull request #7078:
URL: https://github.com/apache/pulsar/pull/7078#issuecomment-635221697
/pulsarbot run-failure-checks
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] aloyszhang edited a comment on pull request #7078: introduce precise topic publish rate limiting
Posted by GitBox <gi...@apache.org>.
aloyszhang edited a comment on pull request #7078:
URL: https://github.com/apache/pulsar/pull/7078#issuecomment-635754644
@rdhabalia Agree with you that `topicPublisherThrottlingTickTimeMillis=1` can provide a morr precise rate limit, but will cause more cpu overhead.
Anohter scenario, if publish rate is very high while publish-rate-limit is quite low, enven
with `topicPublisherThrottlingTickTimeMillis=1`, rate limit still lose effect.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] aloyszhang commented on pull request #7078: introduce precise topic publish rate limiting
Posted by GitBox <gi...@apache.org>.
aloyszhang commented on pull request #7078:
URL: https://github.com/apache/pulsar/pull/7078#issuecomment-636888744
/pulsarbot run-failure-checks
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] aloyszhang commented on pull request #7078: introduce precise topic publish rate limiting
Posted by GitBox <gi...@apache.org>.
aloyszhang commented on pull request #7078:
URL: https://github.com/apache/pulsar/pull/7078#issuecomment-636633379
/pulsarbot run-failure-checks
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org