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

[Branch-2.9] Fix Broker HealthCheck Endpoint Exposes Race Conditions. #14658

Merged
merged 3 commits into from
Mar 14, 2022

Conversation

gaoran10
Copy link
Contributor

@gaoran10 gaoran10 commented Mar 11, 2022

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.

* Make  ``BrokerBase#healthCheck`` to pure async.

* fixes checkstyle

(cherry picked from commit 488fb78)
Copy link
Contributor

@Jason918 Jason918 left a comment

Choose a reason for hiding this comment

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

LGTM

* fix healthcheck v2

* fix failed testHealthCheckupV2 because of error web port

Co-authored-by: gavingaozhangmin <gavingaozhangmin@didiglobal.com>
(cherry picked from commit b38d850)
@codelipenghui codelipenghui merged commit 65af6ce into apache:branch-2.9 Mar 14, 2022
@codelipenghui codelipenghui added the cherry-picked/branch-2.9 Archived: 2.9 is end of life label Mar 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants