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

fix health monitor task #9893

Merged

Conversation

codewithbear
Copy link
Contributor

@codewithbear codewithbear commented Sep 19, 2023

I discovered that HealthMonitoringTask is currently broken. While the periodic task is scheduled according to config, the logic fails to update the CurrentHealthStatus bean.

See example in the added test HealthMonitorTaskSpec.groovy

I found out that the subscriber to reactiveSequence is not getting triggered. It looks like in the onSubscribe we have to explicitly call request method so that HealthIndicators would emit HealthResults

If we fix the above problem with use of request we run into another problem. onComplete in the subscriber reports that the status is UP, even though some checks might be DOWN, and the web endpoints report DOWN. So it looks like incorrect behaviour.

This PR proposes a rewrite in a more clear way and fixing the above problems:

  • first, collect all publishers in a List
  • use Flux.collectList to subscribe to all publishers and collect all results in one list
  • Then we can operate on the final result list in a simple way to check if any status is down and report that. Only if all statuses are up then we report UP

@CLAassistant
Copy link

CLAassistant commented Sep 19, 2023

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@wetted wetted left a comment

Choose a reason for hiding this comment

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

This apparently fixes issues I am having getting a related test to pass on #9895

@graemerocher graemerocher merged commit be96512 into micronaut-projects:4.1.x Sep 21, 2023
6 checks passed
@codewithbear codewithbear deleted the fix-health-monitor-task branch September 21, 2023 08:48
wetted pushed a commit that referenced this pull request Sep 21, 2023
* add test for HealthMonitorTask
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants