Support max-connection and max-connection-per-IP #10754

315157973 · 2021-05-31T13:33:13Z

Motivation

Pulsar supports multi-tenant, and there may be one single user occupying all connections.
Maybe novice user incorrectly used SDK, or we are being attacked.
In order to avoid a single user to affect entire cluster, I think it is important to limit the number of connections.
For simplicity, I did not use radix tree to save IPs.

Modifications

Verifying this change

gaoran10

Great work!

pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ConnectionsLimiter.java

gaoran10

I have some concerns, if connections reach the limitation, the rejected client will re-connect many times and this may confuse users.

Maybe we could make clients receive this connection limit exception and cancel the retry operation, this will also make users know why the client failed to connect with the broker.

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java

315157973 · 2021-05-31T16:19:07Z

I have some concerns, if connections reach the limitation, the rejected client will re-connect many times and this may confuse users.

Maybe we could make clients receive this connection limit exception and cancel the retry operation, this will also make users know why the client failed to connect with the broker.

I will add an exception to prevent the client from retrying. However, clients with lower versions will try to reconnect and eventually time out.

Anonymitaet · 2021-06-01T00:28:24Z

@315157973 thanks for your contribution. For this PR, do we need to update docs?

315157973 · 2021-06-01T02:55:47Z

@315157973 thanks for your contribution. For this PR, do we need to update docs?

The documentation here should be automatically generated, right? There are comments on the code.

Anonymitaet

@315157973 thanks for your contribution. For this PR, do we need to update docs?

The documentation here should be automatically generated, right? There are comments on the code.

Docs need to be added here as well?

conf/broker.conf

Co-authored-by: Yu Liu <50226895+Anonymitaet@users.noreply.github.com>

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ConnectionsLimiter.java

pulsar-broker/src/test/java/org/apache/pulsar/broker/service/BrokerServiceTest.java

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ConnectionsLimiter.java

…tions

Anonymitaet · 2021-06-03T00:58:57Z

@315157973 thanks for your contribution. For this PR, do we need to update docs?

The documentation here should be automatically generated, right? There are comments on the code.

Docs need to be added here as well?

Hi @315157973 any feedback on this? thanks

315157973 · 2021-06-03T02:41:28Z

@315157973 thanks for your contribution. For this PR, do we need to update docs?

The documentation here should be automatically generated, right? There are comments on the code.

Docs need to be added here as well?

Hi @315157973 any feedback on this? thanks

I’m not so sure, it seems to be automatically generated, only 2.7 and previous documents are seen in the code

Anonymitaet · 2021-06-03T03:08:54Z

@315157973 this should be generated automatically, we have an issue requesting this feature before. However, it is not implemented yet.

I find that you've developed a tool to automatically generate documents for pulsar-client, that's awesome!
Would you like to develop a similar one to automatically generate documents for pulsar configuration page? That would be super helpful for both developers and tech writers! Many thanks!

315157973 · 2021-07-02T03:22:58Z

Hello, please take a look again @gaoran10 @cckellogg If there are no other questions, can you approve it?

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ConnectionController.java

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java

gaoran10

LGTM

wolfstudy

Grade Jobs

### Motivation Pulsar supports multi-tenant, and there may be one single user occupying all connections. Maybe novice user incorrectly used SDK, or we are being attacked. In order to avoid a single user to affect entire cluster, I think it is important to limit the number of connections. For simplicity, I did not use radix tree to save IPs. (cherry picked from commit 45caffa)

github-actions · 2021-10-28T04:01:03Z

@315157973:Thanks for your contribution. For this PR, do we need to update docs?
(The PR template contains info about doc, which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)

github-actions · 2021-10-28T04:01:06Z

@315157973:Thanks for your contribution. For this PR, do we need to update docs?
(The PR template contains info about doc, which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)

315157973 · 2021-10-28T05:45:45Z

@Anonymitaet Hello, I remember that we made a tool for automatic document generation before, and the document of this new feature should be automatically generated. Do we still need to write manually now?

Anonymitaet · 2021-10-28T07:46:28Z

@315157973 No.
Sorry above is a bug in doc Bot, please ignore it, we're fixing this issue.

### Motivation Pulsar supports multi-tenant, and there may be one single user occupying all connections. Maybe novice user incorrectly used SDK, or we are being attacked. In order to avoid a single user to affect entire cluster, I think it is important to limit the number of connections. For simplicity, I did not use radix tree to save IPs.

Support max-connection and max-connection-per-IP

08fe6e5

gaoran10 reviewed May 31, 2021

View reviewed changes

315157973 added 3 commits May 31, 2021 22:46

fix doc

6b60dea

code style

80afce5

code style

010c1b0

gaoran10 reviewed May 31, 2021

View reviewed changes

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java Outdated Show resolved Hide resolved

315157973 self-assigned this May 31, 2021

fix incorrect event

675a7b2

let client fail fast

4edc829

315157973 added 3 commits June 1, 2021 11:00

code style

ae47015

code style

63cc873

remove to channelActive to avoid Protocol not available

cb71a0c

Anonymitaet reviewed Jun 1, 2021

View reviewed changes

conf/broker.conf Outdated Show resolved Hide resolved

conf/broker.conf Outdated Show resolved Hide resolved

315157973 and others added 2 commits June 1, 2021 19:36

Update conf/broker.conf

62347a5

Co-authored-by: Yu Liu <50226895+Anonymitaet@users.noreply.github.com>

Update conf/broker.conf

21c2497

Co-authored-by: Yu Liu <50226895+Anonymitaet@users.noreply.github.com>

cckellogg reviewed Jun 1, 2021

View reviewed changes

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ConnectionsLimiter.java Outdated Show resolved Hide resolved

cckellogg reviewed Jun 1, 2021

View reviewed changes

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ConnectionsLimiter.java Outdated Show resolved Hide resolved

315157973 added 6 commits June 2, 2021 01:59

Adopt the above suggestions

07a185b

Merge remote-tracking branch 'origin/max-connections' into max-connec…

50a4e35

…tions

use primitive integer

0ebdf78

code style

3d5ef16

change doc

8c1df91

fix unit test

27131cd

315157973 added 3 commits June 25, 2021 13:06

Support IPV6

6c9bdbe

fix unit test

4adbebc

Merge branch 'master' into max-connections

ee78c6e

codelipenghui approved these changes Jun 25, 2021

View reviewed changes

codelipenghui requested review from cckellogg and gaoran10 June 25, 2021 09:51

gaoran10 reviewed Jul 6, 2021

View reviewed changes

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ConnectionController.java Show resolved Hide resolved

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java Outdated Show resolved Hide resolved

315157973 added 2 commits July 6, 2021 18:54

Differentiate different error messages

3f65d38

style

ce3ce76

gaoran10 approved these changes Jul 7, 2021

View reviewed changes

wolfstudy approved these changes Jul 7, 2021

View reviewed changes

wolfstudy merged commit 45caffa into apache:master Jul 7, 2021

codelipenghui added this to the 2.9.0 milestone Jul 7, 2021

315157973 deleted the max-connections branch July 19, 2021 11:10

codelipenghui added cherry-picked/branch-2.8 Archived: 2.8 is end of life release/2.8.2 labels Oct 28, 2021

github-actions bot added the doc-label-missing label Oct 28, 2021

codelipenghui added doc-label-missing and removed doc-label-missing labels Oct 28, 2021

Anonymitaet added doc Your PR contains doc changes, no matter whether the changes are in markdown or code files. and removed doc-label-missing labels Nov 11, 2021

lhotari mentioned this pull request Apr 29, 2022

[Broker] Fix typo in enum name and handle closing of the channel properly since writeAndFlush is asynchronous #15384

Merged

mattisonchao mentioned this pull request Aug 18, 2022

[feat][proxy] Support proxy limit maximum connections per IP #17167

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support max-connection and max-connection-per-IP #10754

Support max-connection and max-connection-per-IP #10754

315157973 commented May 31, 2021 •

edited

Loading

gaoran10 left a comment

gaoran10 left a comment

315157973 commented May 31, 2021

Anonymitaet commented Jun 1, 2021

315157973 commented Jun 1, 2021

Anonymitaet left a comment

Anonymitaet commented Jun 3, 2021

315157973 commented Jun 3, 2021

Anonymitaet commented Jun 3, 2021 •

edited

Loading

315157973 commented Jul 2, 2021

gaoran10 left a comment

wolfstudy left a comment

github-actions bot commented Oct 28, 2021

github-actions bot commented Oct 28, 2021

315157973 commented Oct 28, 2021

Anonymitaet commented Oct 28, 2021

Support max-connection and max-connection-per-IP #10754

Support max-connection and max-connection-per-IP #10754

Conversation

315157973 commented May 31, 2021 • edited Loading

Motivation

Modifications

Verifying this change

gaoran10 left a comment

Choose a reason for hiding this comment

gaoran10 left a comment

Choose a reason for hiding this comment

315157973 commented May 31, 2021

Anonymitaet commented Jun 1, 2021

315157973 commented Jun 1, 2021

Anonymitaet left a comment

Choose a reason for hiding this comment

Anonymitaet commented Jun 3, 2021

315157973 commented Jun 3, 2021

Anonymitaet commented Jun 3, 2021 • edited Loading

315157973 commented Jul 2, 2021

gaoran10 left a comment

Choose a reason for hiding this comment

wolfstudy left a comment

Choose a reason for hiding this comment

github-actions bot commented Oct 28, 2021

github-actions bot commented Oct 28, 2021

315157973 commented Oct 28, 2021

Anonymitaet commented Oct 28, 2021

315157973 commented May 31, 2021 •

edited

Loading

Anonymitaet commented Jun 3, 2021 •

edited

Loading