Skip to content

Make BrokerBase#healthCheck to pure async#14091

Merged
merlimat merged 2 commits into
apache:masterfrom
mattisonchao:make_healthcheck_async
Feb 5, 2022
Merged

Make BrokerBase#healthCheck to pure async#14091
merlimat merged 2 commits into
apache:masterfrom
mattisonchao:make_healthcheck_async

Conversation

@mattisonchao

Copy link
Copy Markdown
Member

Motivation

Make BrokerBase#healthCheck to pure async.

Modifications

  • change sync method to async.

Verifying this change

  • Make sure that the change passes the CI checks.

Documentation

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

Need to update docs?

  • no-need-doc

@github-actions github-actions Bot added the doc-not-needed Your PR changes do not impact docs label Feb 2, 2022
@merlimat merlimat added the type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages label Feb 2, 2022
@merlimat merlimat merged commit 488fb78 into apache:master Feb 5, 2022
gaoran10 pushed a commit to gaoran10/pulsar that referenced this pull request Mar 11, 2022
* Make  ``BrokerBase#healthCheck`` to pure async.

* fixes checkstyle

(cherry picked from commit 488fb78)
gaoran10 pushed a commit to gaoran10/pulsar that referenced this pull request Mar 11, 2022
* Make  ``BrokerBase#healthCheck`` to pure async.

* fixes checkstyle

(cherry picked from commit 488fb78)
codelipenghui pushed a commit that referenced this pull request Mar 14, 2022
…#14658)

Original PR #14367

Because the PR #14367 is based on PR #14091, so I want to cherry-pick these two PRs to branch-2.9, the PR #13525 is also needed.

---

Fix Issue: #14362

### Motivation

According to relative PR #7724, we will force delete all subscriptions when calling ``healthCheck`` REST API. but it has a race condition when two threads call this API.
Please consider this case:

> Thread A: Clean up all subscriptions, then create a reader.
> Thread B: Prepare to force delete all subscriptions.

So, in this case, the reader of thread A is deleted and then an NPE or other exception occurs.

### Modifications

- Use ``Completable#handle`` to fix problem 1, the reader needs to be closed regardless of whether there is an exception.
- Recheck the subscription after closing reading, and force deletion if it still exists after closing reading.
- Added multi-threaded tests for health checks.
Nicklee007 pushed a commit to Nicklee007/pulsar that referenced this pull request Apr 20, 2022
* Make  ``BrokerBase#healthCheck`` to pure async.

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

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants