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 2021/04/12 07:53:37 UTC
[GitHub] [pulsar] massakam opened a new pull request #10188: [broker] Limit the number of producers/consumers that can connect per topic for each IP address
massakam opened a new pull request #10188:
URL: https://github.com/apache/pulsar/pull/10188
### Motivation
Occasionally, a client application developer makes an implementation mistake and increases the number of producers or consumers infinitely. If this happens, there is concern that all file descriptors on the broker server will be exhausted.
Brokers already have settings such as `maxProducersPerTopic` and `maxConsumersPerTopic`, which allow us to specify a maximum number of clients that can connect per topic. However, if one topic is used by multiple tenants, there is an issue that if one tenant accidentally increases the number of connected clients too much, other tenants will not be able to add new clients.
### Modifications
Enable users to limit the number of producers or consumers which can connect to each topic **with the same IP address**.
--
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] massakam commented on pull request #10188: [broker] Limit the number of producers/consumers that can connect per topic for each IP address
Posted by GitBox <gi...@apache.org>.
massakam commented on pull request #10188:
URL: https://github.com/apache/pulsar/pull/10188#issuecomment-824234679
@315157973 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] 315157973 commented on a change in pull request #10188: [broker] Limit the number of producers/consumers that can connect per topic for each IP address
Posted by GitBox <gi...@apache.org>.
315157973 commented on a change in pull request #10188:
URL: https://github.com/apache/pulsar/pull/10188#discussion_r616577921
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/AbstractTopic.java
##########
@@ -199,14 +220,34 @@ protected boolean isConsumersExceededOnTopic() {
if (maxConsumersPerTopic > 0 && maxConsumersPerTopic <= getNumberOfConsumers()) {
return true;
}
+
+ final int maxSameAddressConsumers = brokerService.pulsar().getConfiguration()
+ .getMaxSameAddressConsumersPerTopic();
Review comment:
I suggest to open a check independently and let the client know clearly that it is because the Consumer of a single IP reaches the upper limit.
Because we will add namespace and topic level Policies in the future. Should not be mixed with max_consumers_per_topic.
Producer is the same.
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentSubscription.java
##########
@@ -1096,5 +1096,18 @@ public boolean checkAndUnblockIfStuck() {
return dispatcher.checkAndUnblockIfStuck();
}
+ @Override
+ public int getNumberOfSameAddressConsumers(final String clientAddress) {
Review comment:
There is still duplicate code here, can we use default to move it to the interface?
It looks stateless.
--
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] 315157973 commented on pull request #10188: [broker] Limit the number of producers/consumers that can connect per topic for each IP address
Posted by GitBox <gi...@apache.org>.
315157973 commented on pull request #10188:
URL: https://github.com/apache/pulsar/pull/10188#issuecomment-818566017
We have recently encountered a similar problem. We need a maximum number of connections based on IP.
--
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 #10188: [broker] Limit the number of producers/consumers that can connect per topic for each IP address
Posted by GitBox <gi...@apache.org>.
codelipenghui merged pull request #10188:
URL: https://github.com/apache/pulsar/pull/10188
--
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] 315157973 commented on a change in pull request #10188: [broker] Limit the number of producers/consumers that can connect per topic for each IP address
Posted by GitBox <gi...@apache.org>.
315157973 commented on a change in pull request #10188:
URL: https://github.com/apache/pulsar/pull/10188#discussion_r611568091
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/Producer.java
##########
@@ -134,6 +136,15 @@ public Producer(Topic topic, TransportCnx cnx, long producerId, String producerN
this.schemaVersion = schemaVersion;
this.accessMode = accessMode;
this.topicEpoch = topicEpoch;
+
+ if (cnx.hasHAProxyMessage()) {
Review comment:
There is duplicate code here, this logic may be abstracted into cnx
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java
##########
@@ -1435,6 +1435,17 @@ public int getNumberOfConsumers() {
return count;
}
+ @Override
+ public int getNumberOfSameAddressConsumers(final String clientAddress) {
Review comment:
Maybe we can abstract the duplicate code. Both `NonPersistentSubscription` and `PersistentSubscription` belong to Subscription.
--
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] massakam commented on pull request #10188: [broker] Limit the number of producers/consumers that can connect per topic for each IP address
Posted by GitBox <gi...@apache.org>.
massakam commented on pull request #10188:
URL: https://github.com/apache/pulsar/pull/10188#issuecomment-819007473
@315157973 Addressed your comments. 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] sijie commented on pull request #10188: [broker] Limit the number of producers/consumers that can connect per topic for each IP address
Posted by GitBox <gi...@apache.org>.
sijie commented on pull request #10188:
URL: https://github.com/apache/pulsar/pull/10188#issuecomment-823060471
@315157973 Can you review this PR again?
--
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