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

Reduce the readFailureBackoff time #12444

Merged
merged 3 commits into from
Oct 22, 2021

Conversation

zymap
Copy link
Member

@zymap zymap commented Oct 21, 2021


Motivation

When reading entries met exception, teh dispatcher will backoff 15s
to start next read. The 15s is unacceptable at most of case for the
consumer. So it's better to reduce the interval.

Modifications

Make the read failure backoff start from 1s.

Verifying this change

  • Make sure that the change passes the CI checks.

(Please pick either of the following options)

This change is a trivial rework / code cleanup without any test coverage.

(or)

This change is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(example:)

  • Added integration tests for end-to-end deployment with large payloads (10MB)
  • Extended integration test for recovery after broker failure

Does this pull request potentially affect one of the following parts:

If yes was chosen, please highlight the changes

  • Dependencies (does it add or upgrade a dependency): (yes / no)
  • The public API: (yes / no)
  • The schema: (yes / no / don't know)
  • The default values of configurations: (yes / no)
  • The wire protocol: (yes / no)
  • The rest endpoints: (yes / no)
  • The admin cli options: (yes / no)
  • Anything that affects deployment: (yes / no / don't know)

Documentation

Check the box below and label this PR (if you have committer privilege).

Need to update docs?

  • doc-required

    (If you need help on updating docs, create a doc issue)

  • no-need-doc

    (Please explain why)

  • doc

    (If this PR contains doc changes)

---

*Motivation*

When reading entries met exception, teh dispatcher will backoff 15s
to start next read. The 15s is unacceptable at most of case for the
consumer. So it's better to reduce the interval.

*Modifications*

Make the read failure backoff start from 1s.
@zymap zymap added doc-not-needed Your PR changes do not impact docs release/2.8.2 release/2.9.1 labels Oct 21, 2021
@zymap zymap added this to the 2.10.0 milestone Oct 21, 2021
@zymap zymap self-assigned this Oct 21, 2021
@hangc0276
Copy link
Contributor

@merlimat Please help review this PR, it decrease the backoff interval, and will increate the retry interval. I'm not sure whether it will introduce other performance impact.

@@ -65,7 +65,7 @@
protected volatile boolean havePendingRead = false;

protected volatile int readBatchSize;
protected final Backoff readFailureBackoff = new Backoff(15, TimeUnit.SECONDS,
protected final Backoff readFailureBackoff = new Backoff(1, TimeUnit.SECONDS,
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we make it configurable at the broker.conf? In some cases, May 1 sec is still too long.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure

@hangc0276 hangc0276 merged commit 99c90a5 into apache:master Oct 22, 2021
zymap added a commit that referenced this pull request Oct 22, 2021
### Motivation
When reading entries met exception, the dispatcher will backoff 15s
to start next read. The 15s is unacceptable at most of case for the
consumer. So it's better to reduce the interval.

### Modifications
Make the read failure backoff start from 1s.

(cherry picked from commit 99c90a5)
@zymap zymap added the cherry-picked/branch-2.8 Archived: 2.8 is end of life label Oct 22, 2021
eolivelli pushed a commit to eolivelli/pulsar that referenced this pull request Nov 29, 2021
### Motivation
When reading entries met exception, the dispatcher will backoff 15s
to start next read. The 15s is unacceptable at most of case for the
consumer. So it's better to reduce the interval.

### Modifications
Make the read failure backoff start from 1s.
codelipenghui pushed a commit that referenced this pull request Dec 20, 2021
### Motivation
When reading entries met exception, the dispatcher will backoff 15s
to start next read. The 15s is unacceptable at most of case for the
consumer. So it's better to reduce the interval.

### Modifications
Make the read failure backoff start from 1s.

(cherry picked from commit 99c90a5)
@codelipenghui codelipenghui added the cherry-picked/branch-2.9 Archived: 2.9 is end of life label Dec 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/config cherry-picked/branch-2.8 Archived: 2.8 is end of life cherry-picked/branch-2.9 Archived: 2.9 is end of life doc-not-needed Your PR changes do not impact docs release/2.8.2 release/2.9.2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants