Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Avoid duplicated disconnecting producer when after add entry failed. #11741

Merged
merged 1 commit into from Aug 23, 2021
Merged

Avoid duplicated disconnecting producer when after add entry failed. #11741

merged 1 commit into from Aug 23, 2021

Conversation

codelipenghui
Copy link
Contributor

@codelipenghui codelipenghui commented Aug 22, 2021 •
edited

Motivation

Currently, if encounter the add entry failure, will call producer.disconnect() multiple times during the disconnecting the producer which will add many disconnect producer tasks to the EventLoop.

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

Lines 563 to 572 in da71ec2

public CompletableFuture<Void> disconnect() {
if (!closeFuture.isDone()) {
log.info("Disconnecting producer: {}", this);
cnx.execute(() -> {
cnx.closeProducer(this);
closeNow(true);
});
}
return closeFuture;
}

Changes

  1. Added isDisconnecting state for the producer, if the producer is in isDisconnecting state, skip the disconnect operation
  2. Create new future list only the topic have producers to reduce the heap allocation

Verify

Added test to cover disconnecting the producer multiple times, but the EventLoop only execute one time.

@codelipenghui
Avoid duplicated disconnecting producer when after add entry failed.
125c3a3 
### Motivation

Currently, if encounter the add entry failure, will call producer.disconnect() multiple times during the disconnecting the producer
which will add many disconnect producer tasks to the EventLoop.

### Changes

1. Added isDisconnecting state for the producer, if the producer in isDisconnecting state, skip the disconnect operation
2. Create new future list only the topic have producers to reduce the heap allocation

### Verify

Added test to cover disconnecting the producer multiple times, but the EventLoop only execute one time.
@codelipenghui codelipenghui self-assigned this Aug 22, 2021
@codelipenghui codelipenghui added this to the 2.9.0 milestone Aug 22, 2021
@codelipenghui codelipenghui added release/2.8.1 doc-not-needed Your PR changes do not impact docs type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages labels Aug 22, 2021
eolivelli
eolivelli approved these changes Aug 23, 2021
View reviewed changes
Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm

Good catch

@merlimat merlimat merged commit 49c0796 into apache:master Aug 23, 2021
@codelipenghui codelipenghui deleted the penghui/avoid-dup-disconnect-producer branch August 24, 2021 02:12
hangc0276 pushed a commit that referenced this pull request Aug 25, 2021
@codelipenghui @hangc0276
Avoid duplicated disconnecting producer when after add entry failed. (#…
9c2888f 
…11741)

Currently, if encounter the add entry failure, will call producer.disconnect() multiple times during the disconnecting the producer
which will add many disconnect producer tasks to the EventLoop.

1. Added isDisconnecting state for the producer, if the producer in isDisconnecting state, skip the disconnect operation
2. Create new future list only the topic have producers to reduce the heap allocation

Added test to cover disconnecting the producer multiple times, but the EventLoop only execute one time.

(cherry picked from commit 49c0796)
@hangc0276 hangc0276 added the cherry-picked/branch-2.8 Archived: 2.8 is end of life label Aug 25, 2021
bharanic-dev pushed a commit to bharanic-dev/pulsar that referenced this pull request Mar 18, 2022
@codelipenghui @ciaocloud
Avoid duplicated disconnecting producer when after add entry failed. (a…
b325db3 
…pache#11741)

### Motivation

Currently, if encounter the add entry failure, will call producer.disconnect() multiple times during the disconnecting the producer
which will add many disconnect producer tasks to the EventLoop.

### Changes

1. Added isDisconnecting state for the producer, if the producer in isDisconnecting state, skip the disconnect operation
2. Create new future list only the topic have producers to reduce the heap allocation

### Verify

Added test to cover disconnecting the producer multiple times, but the EventLoop only execute one time.
@codelipenghui codelipenghui restored the penghui/avoid-dup-disconnect-producer branch May 17, 2022 01:23
@codelipenghui codelipenghui deleted the penghui/avoid-dup-disconnect-producer branch May 17, 2022 01:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@merlimat merlimat merlimat approved these changes

@lhotari lhotari lhotari approved these changes

@eolivelli eolivelli eolivelli approved these changes

@315157973 315157973 315157973 approved these changes

@cckellogg cckellogg Awaiting requested review from cckellogg

@hangc0276 hangc0276 Awaiting requested review from hangc0276

@gaoran10 gaoran10 Awaiting requested review from gaoran10

@sijie sijie Awaiting requested review from sijie

Assignees

@codelipenghui codelipenghui

Labels
cherry-picked/branch-2.8 Archived: 2.8 is end of life doc-not-needed Your PR changes do not impact docs release/2.8.1 type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages
Projects
None yet
Milestone
2.9.0
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@codelipenghui @merlimat @lhotari @eolivelli @315157973 @hangc0276