[improve][broker] System topic writer/reader connection not counted. #18369

Technoboy- · 2022-11-07T12:58:15Z

Master Issue: #18364

Motivation

It's better not to count the system topic writer/reader connection.

Documentation

doc
doc-required
doc-not-needed
doc-complete

codecov-commenter · 2022-11-07T13:11:20Z

Codecov Report

Merging #18369 (53710c1) into master (27186a1) will increase coverage by 1.71%.
The diff coverage is 63.63%.

@@             Coverage Diff              @@
##             master   #18369      +/-   ##
============================================
+ Coverage     45.61%   47.33%   +1.71%     
+ Complexity    10728    10479     -249     
============================================
  Files           752      698      -54     
  Lines         72521    68070    -4451     
  Branches       7791     7279     -512     
============================================
- Hits          33083    32221     -862     
+ Misses        35769    32263    -3506     
+ Partials       3669     3586      -83

Flag	Coverage Δ
unittests	`47.33% <63.63%> (+1.71%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...sar/broker/service/persistent/PersistentTopic.java	`62.04% <ø> (-0.10%)`	⬇️
...rg/apache/pulsar/broker/service/AbstractTopic.java	`65.26% <63.63%> (+0.55%)`	⬆️
...java/org/apache/pulsar/proxy/stats/TopicStats.java	`58.82% <0.00%> (-41.18%)`	⬇️
.../apache/pulsar/broker/loadbalance/LoadManager.java	`61.11% <0.00%> (-16.67%)`	⬇️
...tent/NonPersistentDispatcherMultipleConsumers.java	`40.74% <0.00%> (-12.35%)`	⬇️
...apache/pulsar/broker/service/TopicListService.java	`42.62% <0.00%> (-12.30%)`	⬇️
.../org/apache/bookkeeper/mledger/impl/EntryImpl.java	`69.62% <0.00%> (-11.27%)`	⬇️
...keeper/mledger/impl/cache/RangeEntryCacheImpl.java	`45.79% <0.00%> (-6.31%)`	⬇️
...pulsar/broker/service/PulsarCommandSenderImpl.java	`72.30% <0.00%> (-5.71%)`	⬇️
...g/apache/pulsar/client/impl/ConnectionHandler.java	`50.00% <0.00%> (-5.32%)`	⬇️
... and 135 more

grssam · 2022-11-07T13:14:39Z

@Technoboy- Thank you for the PR.
It's weird that such a generic issue, and we never faced it in our production system till now.. Is this a regression in 2.9.3? We were previously on 2.7.4 up until september. I tried to look at differences in 2.7 vs 2.9 wrt this particular context and could not see any sort of regression; Does it mean that the problem existed in 2.7.* as well?

Technoboy- · 2022-11-08T02:02:30Z

@Technoboy- Thank you for the PR. It's weird that such a generic issue, and we never faced it in our production system till now.. Is this a regression in 2.9.3? We were previously on 2.7.4 up until september. I tried to look at differences in 2.7 vs 2.9 wrt this particular context and could not see any sort of regression; Does it mean that the problem existed in 2.7.* as well?

yes, it existed in branch-2.7.

pulsar-broker/src/test/java/org/apache/pulsar/broker/systopic/PartitionedSystemTopicTest.java

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

grssam · 2022-11-08T11:50:09Z

@Technoboy- Thank you for the PR. It's weird that such a generic issue, and we never faced it in our production system till now.. Is this a regression in 2.9.3? We were previously on 2.7.4 up until september. I tried to look at differences in 2.7 vs 2.9 wrt this particular context and could not see any sort of regression; Does it mean that the problem existed in 2.7.* as well?

yes, it existed in branch-2.7.

Thanks. It's weird that no one ever faced this issue before.. even we have been running for over an year without facing this issue..

codelipenghui

Do we need to change method isConsumersExceededOnTopic? We have max subscriptions and max consumers limitation for a topic.

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

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

pulsar-broker/src/test/java/org/apache/pulsar/broker/systopic/PartitionedSystemTopicTest.java

mattisonchao

Please apply @codelipenghui's comments.

Technoboy- · 2022-11-23T04:03:22Z

Do we need to change method isConsumersExceededOnTopic? We have max subscriptions and max consumers limitation for a topic.

yes, already fixed. please help review, thanks.

grssam · 2022-11-25T15:07:16Z

Please cherry-pick this and #18603 to 2.9 branch as well.

…18603) This PR is a supplement to #18369. - `AbstractTopic.isSameAddressProducersExceeded()` - `AbstractBaseDispatcher.isConsumersExceededOnSubscription()`

…pache#18603) This PR is a supplement to apache#18369. - `AbstractTopic.isSameAddressProducersExceeded()` - `AbstractBaseDispatcher.isConsumersExceededOnSubscription()`

…18369) (cherry picked from commit b219cca)

…18603) This PR is a supplement to #18369. - `AbstractTopic.isSameAddressProducersExceeded()` - `AbstractBaseDispatcher.isConsumersExceededOnSubscription()` (cherry picked from commit a2fb562)

…18369)

…18603) This PR is a supplement to #18369. - `AbstractTopic.isSameAddressProducersExceeded()` - `AbstractBaseDispatcher.isConsumersExceededOnSubscription()`

…18369)

…18603) This PR is a supplement to #18369. - `AbstractTopic.isSameAddressProducersExceeded()` - `AbstractBaseDispatcher.isConsumersExceededOnSubscription()`

…pache#18369)

…pache#18603) This PR is a supplement to apache#18369. - `AbstractTopic.isSameAddressProducersExceeded()` - `AbstractBaseDispatcher.isConsumersExceededOnSubscription()`

…pache#18369) (cherry picked from commit 51cac6b)

…pache#18603) This PR is a supplement to apache#18369. - `AbstractTopic.isSameAddressProducersExceeded()` - `AbstractBaseDispatcher.isConsumersExceededOnSubscription()` (cherry picked from commit b33bff9)

…pache#18369)

…pache#18603) This PR is a supplement to apache#18369. - `AbstractTopic.isSameAddressProducersExceeded()` - `AbstractBaseDispatcher.isConsumersExceededOnSubscription()`

…pache#18369) (cherry picked from commit 51cac6b)

…pache#18603) This PR is a supplement to apache#18369. - `AbstractTopic.isSameAddressProducersExceeded()` - `AbstractBaseDispatcher.isConsumersExceededOnSubscription()` (cherry picked from commit b33bff9)

System topic writer/reader connection not counted.

2c308a0

Technoboy- added type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages area/broker ready-to-test labels Nov 7, 2022

github-actions bot added the doc-not-needed Your PR changes do not impact docs label Nov 7, 2022

Technoboy- self-assigned this Nov 7, 2022

Technoboy- added this to the 2.12.0 milestone Nov 7, 2022

mattisonchao reviewed Nov 8, 2022

View reviewed changes

pulsar-broker/src/test/java/org/apache/pulsar/broker/systopic/PartitionedSystemTopicTest.java Outdated Show resolved Hide resolved

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

update test.

421fe1d

Technoboy- added 2 commits November 8, 2022 21:15

apply comment.

26d3bd3

fix test.

997b050

Technoboy- closed this Nov 17, 2022

Technoboy- reopened this Nov 17, 2022

codelipenghui reviewed Nov 22, 2022

View reviewed changes

Technoboy- added 2 commits November 22, 2022 10:18

apply comment.

ce0b2eb

apply comment.

ef991c8

mattisonchao approved these changes Nov 23, 2022

View reviewed changes

apply comment.

a9de004

remove blank.

53710c1

codelipenghui approved these changes Nov 23, 2022

View reviewed changes

codelipenghui merged commit b219cca into apache:master Nov 23, 2022

codelipenghui added release/2.11.1 release/2.10.3 release/2.9.5 labels Nov 23, 2022

yuruguo mentioned this pull request Nov 24, 2022

[improve][broker] System topic writer/reader connection not counted #18603

Merged

4 tasks

congbobo184 added release/2.9.4 and removed release/2.9.5 labels Dec 6, 2022

congbobo184 pushed a commit that referenced this pull request Dec 6, 2022

[improve][broker] System topic writer/reader connection not counted. (#…

1d01714

…18369) (cherry picked from commit b219cca)

congbobo184 added the cherry-picked/branch-2.9 Archived: 2.9 is end of life label Dec 7, 2022

Technoboy- mentioned this pull request Dec 8, 2022

[Bug] Unable to init topic policies as maximum subscription limit reached on __change_events topic #18364

Closed

2 tasks

Technoboy- added the cherry-picked/branch-2.10 label Dec 8, 2022

Technoboy- added a commit that referenced this pull request Dec 8, 2022

[improve][broker] System topic writer/reader connection not counted. (#…

b637d54

…18369)

Technoboy- added release/2.11.0 cherry-picked/branch-2.11 and removed release/2.11.1 labels Dec 8, 2022

Technoboy- added a commit that referenced this pull request Dec 8, 2022

[improve][broker] System topic writer/reader connection not counted. (#…

51cac6b

…18369)

lifepuzzlefun pushed a commit to lifepuzzlefun/pulsar that referenced this pull request Dec 9, 2022

[improve][broker] System topic writer/reader connection not counted. (a…

0f3e2f1

…pache#18369)

nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request Jan 10, 2023

[improve][broker] System topic writer/reader connection not counted. (a…

c6ad205

…pache#18369) (cherry picked from commit 51cac6b)

lifepuzzlefun pushed a commit to lifepuzzlefun/pulsar that referenced this pull request Jan 10, 2023

[improve][broker] System topic writer/reader connection not counted. (a…

383bf16

…pache#18369)

nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request Jan 11, 2023

[improve][broker] System topic writer/reader connection not counted. (a…

e39c370

…pache#18369) (cherry picked from commit 51cac6b)

Technoboy- deleted the producer-consumer-exceed branch September 14, 2023 11:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[improve][broker] System topic writer/reader connection not counted. #18369

[improve][broker] System topic writer/reader connection not counted. #18369

[improve][broker] System topic writer/reader connection not counted. #18369

[improve][broker] System topic writer/reader connection not counted. #18369

Conversation

Motivation

Documentation

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment