Skip to content

Use get instead of join to avoid getting stuck - #11597

Merged
codelipenghui merged 4 commits into
apache:masterfrom
315157973:get-instead-of-join
Aug 9, 2021
Merged

Use get instead of join to avoid getting stuck#11597
codelipenghui merged 4 commits into
apache:masterfrom
315157973:get-instead-of-join

Conversation

@315157973

Copy link
Copy Markdown
Contributor

Motivation

join() will not time out, when network jitter occurs, it is easy to cause the thread to get stuck forever

Modifications

Use get(timeout) instead of join() to avoid getting stuck

@315157973 315157973 self-assigned this Aug 8, 2021
@315157973 315157973 added the doc-not-needed Your PR changes do not impact docs label Aug 8, 2021

@eolivelli eolivelli left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Overall the patch LGTM

But I left few comments

@eolivelli eolivelli left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Lgtm

@codelipenghui codelipenghui added this to the 2.9.0 milestone Aug 9, 2021

@gaoran10 gaoran10 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@codelipenghui
codelipenghui merged commit d57c1d2 into apache:master Aug 9, 2021
LeBW pushed a commit to LeBW/pulsar that referenced this pull request Aug 9, 2021
@hangc0276

Copy link
Copy Markdown
Contributor

This PR depend on #11265 , which is a feature. we move this pr to 2.8.2 first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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.2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants