fix(service-operation): honor max_retries in uptime service checks#232
Open
Flegma wants to merge 1 commit into
Open
fix(service-operation): honor max_retries in uptime service checks#232Flegma wants to merge 1 commit into
Flegma wants to merge 1 commit into
Conversation
The services collection stores max_retries (UI "Retry Attempts") and the Go Service struct already deserializes it, but performCheck ran exactly one attempt and marked the service down on the first failure. The field was effectively write-only: notifications fired on a single slow or dropped probe. Retry the check up to max_retries times (2s apart, per-attempt timeout unchanged) and only mark the service down after every attempt fails. This mirrors the legacy frontend checker (httpChecker.ts used service.max_retries || 3). Unset or invalid values fall back to 3; values above 10 are clamped. Checks run in the per-service monitor goroutine, so retry sleeps never delay other services. Adds the module's first tests (monitoring/checker_test.go).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Make the uptime checker honor the per-service Retry Attempts setting (
services.max_retries). A service is now only markeddownafter all attempts in a check cycle fail, instead of after the first failed probe.Closes #231. Fixes the behavior reported in #180 (closed as duplicate of #176) and complements the notification strategy discussed in #176.
Why
max_retriesis configurable in the UI, stored by the frontend, and already deserialized into the GoServicestruct, but nothing underserver/ever read it.performCheckran exactly one attempt per cycle, so a single slow or dropped probe flapped the service todownand fired notifications. The legacy frontend checker honored the field (httpChecker.ts:service.max_retries || 3), so this restores parity lost in the migration to the Go microservice.Real-world impact: on our production instance (interval 300s, Retry Attempts 5, 10s timeout) one probe crossing the timeout during a nightly backup produced a false "DOWN, then UP 4.5 minutes later" alert, with exactly one probe request visible in the origin access log per incident.
How
In
server/service-operation/monitoring/checker.go:performChecknow callsrunCheckAttempts, which executes up tomax_retriesattempts (2s apart, per-attempt timeout unchanged at 10s) and returns on the first success, otherwise the final attempt's outcome.executeCheck(a plain function, so it is directly testable). Unknown service types keep the exact current behavior: log and return without a status update.normalizeMaxRetriesfalls back to 3 (the UI default and legacy frontend default) when the field is unset or invalid, and clamps to 10 defensively.Notes on behavior and scope:
startMonitor), so retry sleeps only delay that service's own loop, never other services. Worst case with the UI maximum (5 attempts): about 58s inside one cycle.max_retriesmeans 3 attempts, which is what the UI has claimed all along. Users who want single-shot semantics can set Retry Attempts to 1.downresult is the final attempt's, matching current semantics.Testing
Added
monitoring/checker_test.go(first tests in the module):max_retriesnormalization (unset, negative, in-range, above cap).upand the server saw exactly 3 requests.downafter exactlymax_retriesrequests.executeCheckandrunCheckAttempts.Ran with Go 1.21 (matching
go.mod):Changed files are
gofmtclean. Heads-up unrelated to this PR:go build ./...ondevelopalready fails inping/icmp.go(a pre-existing type error in a package nothing imports); the main binary target builds fine before and after this change, so I left that package untouched to keep this PR focused.