[improve] [broker] Let the producer request success at the first time if the previous one is inactive #21220

poorbarcode · 2023-09-21T09:45:25Z

Motivation

If a producer establishes a new connection when it is reconnecting, while the previous connection is now inactive, the initial request made on the new connection will fail. This failure will trigger the topic of cleaning up the inactive producers. However, upon making a second request, the producer will be able to successfully establish a connection and proceed with the operation.

Modifications

Make the initial request made on the new connection success.

Documentation

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

Matching PR in forked repository

PR in forked repository: x

github-actions · 2023-10-22T01:48:36Z

The pr had no activity for 30 days, mark with Stale label.

… if the previous is inactive

codelipenghui · 2023-11-01T09:55:02Z

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

+                                "Producer with name '" + newProducer.getProducerName()
+                                        + "' is already connected to topic"));
+                    } else {
+                        return internalAddProducer(newProducer);


It's better to add a comment here to explain why we should take a recursive call.

And it also should be BUG before, right?

It's better to add a comment here to explain why we should take a recursive call.

Done

And it also should be BUG before, right?

Yes, the issue is that the producer creation will fail if a same-name producer has already closed on the client side.

codecov-commenter · 2023-11-02T03:44:04Z

Codecov Report

❌ Patch coverage is 75.00000% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.23%. Comparing base (bc84721) to head (f7aa64f).
⚠️ Report is 1585 commits behind head on master.

Files with missing lines	Patch %	Lines
...rg/apache/pulsar/broker/service/AbstractTopic.java	75.00%	4 Missing and 1 partial ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             master   #21220       +/-   ##
=============================================
+ Coverage     36.74%   73.23%   +36.48%     
- Complexity      375    32607    +32232     
=============================================
  Files          1713     1890      +177     
  Lines        130808   140428     +9620     
  Branches      14256    15434     +1178     
=============================================
+ Hits          48068   102841    +54773     
+ Misses        76375    29498    -46877     
- Partials       6365     8089     +1724

Flag	Coverage Δ
inttests	`24.16% <20.00%> (-0.09%)`	⬇️
systests	`24.69% <20.00%> (-0.05%)`	⬇️
unittests	`72.52% <75.00%> (+40.66%)`	⬆️

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

Files with missing lines	Coverage Δ
...rg/apache/pulsar/broker/service/AbstractTopic.java	`87.14% <75.00%> (+28.25%)`	⬆️

... and 1454 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

… if the previous one is inactive (apache#21220) ### Motivation If a producer establishes a new connection when it is reconnecting, while the previous connection is now inactive, the initial request made on the new connection will fail. This failure will trigger the topic of cleaning up the inactive producers. However, upon making a second request, the producer will be able to successfully establish a connection and proceed with the operation. ### Modifications Make the initial request made on the new connection success.

… if the previous one is inactive (#21220) ### Motivation If a producer establishes a new connection when it is reconnecting, while the previous connection is now inactive, the initial request made on the new connection will fail. This failure will trigger the topic of cleaning up the inactive producers. However, upon making a second request, the producer will be able to successfully establish a connection and proceed with the operation. ### Modifications Make the initial request made on the new connection success.

AnonHxy · 2023-11-28T11:32:43Z

There are conflicts when cherry pick this patch to branch-3.1, would you please help do it @poorbarcode

… if the previous one is inactive (#21220) If a producer establishes a new connection when it is reconnecting, while the previous connection is now inactive, the initial request made on the new connection will fail. This failure will trigger the topic of cleaning up the inactive producers. However, upon making a second request, the producer will be able to successfully establish a connection and proceed with the operation. Make the initial request made on the new connection success. (cherry picked from commit 711b621)

poorbarcode · 2023-11-28T15:29:33Z

There are conflicts when cherry pick this patch to branch-3.1, would you please help do it @poorbarcode

Done

… if the previous one is inactive (#21220) If a producer establishes a new connection when it is reconnecting, while the previous connection is now inactive, the initial request made on the new connection will fail. This failure will trigger the topic of cleaning up the inactive producers. However, upon making a second request, the producer will be able to successfully establish a connection and proceed with the operation. Make the initial request made on the new connection success. (cherry picked from commit 40f94d5)

…rst time if the previous one is inactive (#21220)" This reverts commit 918e746.

… if the previous one is inactive (#21220) If a producer establishes a new connection when it is reconnecting, while the previous connection is now inactive, the initial request made on the new connection will fail. This failure will trigger the topic of cleaning up the inactive producers. However, upon making a second request, the producer will be able to successfully establish a connection and proceed with the operation. Make the initial request made on the new connection success. (cherry picked from commit 40f94d5)

… if the previous one is inactive (#21220) ### Motivation If a producer establishes a new connection when it is reconnecting, while the previous connection is now inactive, the initial request made on the new connection will fail. This failure will trigger the topic of cleaning up the inactive producers. However, upon making a second request, the producer will be able to successfully establish a connection and proceed with the operation. ### Modifications Make the initial request made on the new connection success. (cherry picked from commit 40f94d5)

… if the previous one is inactive (apache#21220) If a producer establishes a new connection when it is reconnecting, while the previous connection is now inactive, the initial request made on the new connection will fail. This failure will trigger the topic of cleaning up the inactive producers. However, upon making a second request, the producer will be able to successfully establish a connection and proceed with the operation. Make the initial request made on the new connection success. (cherry picked from commit 40f94d5)

…rst time if the previous one is inactive (apache#21220)" This reverts commit 918e746.

… if the previous one is inactive (apache#21220) If a producer establishes a new connection when it is reconnecting, while the previous connection is now inactive, the initial request made on the new connection will fail. This failure will trigger the topic of cleaning up the inactive producers. However, upon making a second request, the producer will be able to successfully establish a connection and proceed with the operation. Make the initial request made on the new connection success. (cherry picked from commit 40f94d5)

… if the previous one is inactive (apache#21220) If a producer establishes a new connection when it is reconnecting, while the previous connection is now inactive, the initial request made on the new connection will fail. This failure will trigger the topic of cleaning up the inactive producers. However, upon making a second request, the producer will be able to successfully establish a connection and proceed with the operation. Make the initial request made on the new connection success. (cherry picked from commit 711b621)

poorbarcode requested review from Technoboy-, codelipenghui and liangyepianzhou September 21, 2023 09:45

github-actions bot added the doc-not-needed Your PR changes do not impact docs label Sep 21, 2023

poorbarcode requested a review from michaeljmarshall September 21, 2023 09:46

poorbarcode mentioned this pull request Sep 21, 2023

[fix] [broker] Make specified producer could override the previous one #21155

Merged

4 tasks

poorbarcode self-assigned this Sep 21, 2023

poorbarcode added release/3.0.2 release/2.11.3 release/2.10.6 category/reliability The function does not work properly in certain specific environments or failures. e.g. data lost labels Sep 21, 2023

poorbarcode added this to the 3.2.0 milestone Sep 21, 2023

github-actions bot added the Stale label Oct 22, 2023

poorbarcode added release/3.0.3 and removed release/3.0.2 labels Oct 24, 2023

shibd added release/2.11.4 and removed release/2.11.3 labels Oct 26, 2023

poorbarcode added the type/bug The PR fixed a bug or issue reported a bug label Oct 27, 2023

[improve] [broker] Let the producer request success at the first time…

8710eb5

… if the previous is inactive

poorbarcode force-pushed the improve/producer_success_at_first_time branch from 3709b39 to 8710eb5 Compare October 31, 2023 09:22

remove unnecessary code

585ae8b

codelipenghui approved these changes Nov 1, 2023

View reviewed changes

add code comment to expain why add a recursive call

f7aa64f

poorbarcode closed this Nov 1, 2023

poorbarcode reopened this Nov 1, 2023

liangyepianzhou approved these changes Nov 2, 2023

View reviewed changes

poorbarcode merged commit 40f94d5 into apache:master Nov 2, 2023

Technoboy- added cherry-picked/branch-3.0 release/3.1.2 labels Nov 23, 2023

poorbarcode deleted the improve/producer_success_at_first_time branch November 28, 2023 15:29

poorbarcode added the cherry-picked/branch-3.1 label Nov 28, 2023

poorbarcode added a commit that referenced this pull request Nov 28, 2023

Revert "[improve] [broker] Let the producer request success at the fi…

5cd45ae

…rst time if the previous one is inactive (#21220)" This reverts commit 918e746.

poorbarcode added cherry-picked/branch-2.10 cherry-picked/branch-2.11 labels Nov 28, 2023

nodece pushed a commit to ascentstream/pulsar that referenced this pull request Dec 2, 2023

Revert "[improve] [broker] Let the producer request success at the fi…

ba0e37f

…rst time if the previous one is inactive (apache#21220)" This reverts commit 918e746.

shibd added release/2.11.3 and removed release/2.11.4 labels Dec 20, 2023

Technoboy- mentioned this pull request Jan 10, 2024

[Bug] [worker] ProducerFencedException：Topic has an existing exclusive producer #21809

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[improve] [broker] Let the producer request success at the first time if the previous one is inactive #21220

[improve] [broker] Let the producer request success at the first time if the previous one is inactive #21220

Uh oh!

poorbarcode commented Sep 21, 2023

Uh oh!

github-actions bot commented Oct 22, 2023

Uh oh!

codelipenghui Nov 1, 2023

Uh oh!

codelipenghui Nov 1, 2023

Uh oh!

poorbarcode Nov 1, 2023

Uh oh!

codecov-commenter commented Nov 2, 2023 •

edited

Loading

Uh oh!

AnonHxy commented Nov 28, 2023

Uh oh!

poorbarcode commented Nov 28, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

[improve] [broker] Let the producer request success at the first time if the previous one is inactive #21220

[improve] [broker] Let the producer request success at the first time if the previous one is inactive #21220

Uh oh!

Conversation

poorbarcode commented Sep 21, 2023

Motivation

Modifications

Documentation

Matching PR in forked repository

Uh oh!

github-actions bot commented Oct 22, 2023

Uh oh!

codelipenghui Nov 1, 2023

Choose a reason for hiding this comment

Uh oh!

codelipenghui Nov 1, 2023

Choose a reason for hiding this comment

Uh oh!

poorbarcode Nov 1, 2023

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Nov 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

AnonHxy commented Nov 28, 2023

Uh oh!

poorbarcode commented Nov 28, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

codecov-commenter commented Nov 2, 2023 •

edited

Loading