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.

Sign up for GitHub

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

Jump to bottom

Adds a basic implementation of Recheck Interval into the health check service. #10610

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Adds a basic implementation of Recheck Interval into the health check service. #10610

wants to merge 2 commits into from

Conversation

sswastik02
Copy link

@sswastik02 sswastik02 commented Apr 12, 2024 •
edited

What does this PR do?

This PR Addresses the issue where the interval for health checks remains static even when a service starts failing. This lack of adaptability may result in unreliable detection of issues on time. By implementing a recheck interval, we ensure that the health status of a service is re-evaluated at regular intervals, especially when it fails, thereby improving the accuracy and responsiveness of issue detection.

Motivation

Aims to improve the reliability and responsiveness of issue detection, particularly in scenarios where services may intermittently fail or experience fluctuations in health status.

More

  • Added/updated tests
  • Added/updated documentation

Additional Notes

Utilization of providers is yet to be implemented. Future work will focus on integrating this functionality to further enhance the flexibility and extensibility of the health check system.

Fixes #6550

@traefiker traefiker added this to the 3.0 milestone Apr 12, 2024
@sswastik02
Copy link
Author

Please let me know if I made the PR correctly, or if any changes are needed.

@nmengin nmengin added kind/enhancement a new or improved feature. and removed status/0-needs-triage labels Apr 15, 2024
@kevinpollet kevinpollet changed the base branch from v3.0 to master April 15, 2024 12:20
@nmengin nmengin modified the milestones: 3.0, next Apr 15, 2024
@kevinpollet kevinpollet self-assigned this Apr 18, 2024
@sswastik02
Copy link
Author

Hi @kevinpollet ,

I'm encountering issues with failing tests due to the interface change from StatusSetter to StatusHandler. Specifically, for the recheck interval feature to function properly, I need the Getter Method on the status to determine when to switch to recheck and when to revert.

I'd appreciate your input on how best to address this. One option is to modify the interface to eliminate the need for the getter method, perhaps by having SetStatus return the original status before modification. Your suggestions on ensuring compatibility with other services implementing this interface would be invaluable.

Thank you.

@sswastik02
Copy link
Author

Another possibility is to consider expanding the tests to include this new field, which I believe will become necessary regardless. Your guidance on this matter would be greatly appreciated, @kevinpollet.

@sswastik02 sswastik02 force-pushed the master branch 3 times, most recently from a74d30b to af3d0b9 Compare May 19, 2024 14:36
@sswastik02
Copy link
Author

Hi @kevinpollet ,
Apologies for the delay in PR, requesting you to approve the workflows to run.

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

@kevinpollet kevinpollet Awaiting requested review from kevinpollet

At least 1 approving review is required to merge this pull request.

Assignees

@kevinpollet kevinpollet

Labels
area/healthcheck kind/enhancement a new or improved feature. size/L status/2-needs-review
Projects
None yet
Milestone
next
Development

Successfully merging this pull request may close these issues.

Support recheck interval for health check
5 participants
@sswastik02 @kevinpollet @nmengin @rtribotte @traefiker