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

feat: Added retry logic to remote api request #760

Closed

Conversation

dmitryanchikov
Copy link

@dmitryanchikov dmitryanchikov commented Jun 22, 2022

PR Summary

  • New error type is added ErrRemoteUnavailable. This error is supposed to be used for cases when Remote API is unavailable, e.g. repetitively returns following errors: 401, 502, 503, 504, transport errors, timeouts
  • New config settings are added to Remote section in checker, api and notifier configs:
    • RetrySeconds json: retry_seconds - defines time intervals in seconds for retry calls for errors above, ex.: '1 10 30', '1 1
    • HealthCheckTimeout json: health_check_timeout - defines timeout for calls for health check, ex.: 5s
    • HealthCheckRetrySeconds json: health_check_retry_seconds defines time intervals in seconds for health check retry calls, ex.: 5 5
  • New method makeRequestWithRetries is added to perform calls with retries in case of getting errors from the list above. This method is applied in both health check call and fetching trigger metrics call.

Future work

There will be follow-up PR for including logic of handling of ErrRemoteUnavailable error in trigger check processing and PR for adding new metric for monitoring of graphite unavailability

@dmitryanchikov dmitryanchikov self-assigned this Jun 22, 2022
@dmitryanchikov dmitryanchikov requested a review from a team as a code owner June 22, 2022 09:33
@dmitryanchikov dmitryanchikov force-pushed the feature/graphite-unavailability-retriable-calls branch 6 times, most recently from 3ee81e6 to 4ed8d18 Compare June 23, 2022 09:58
@dmitryanchikov dmitryanchikov force-pushed the feature/graphite-unavailability-retriable-calls branch 4 times, most recently from 0bc0ee5 to bcc4b92 Compare July 6, 2022 08:09
local/checker.yml Show resolved Hide resolved
cmd/config.go Show resolved Hide resolved
@dmitryanchikov dmitryanchikov force-pushed the feature/graphite-unavailability-retriable-calls branch 2 times, most recently from c694629 to a326352 Compare July 6, 2022 11:51
@kissken kissken force-pushed the feature/graphite-unavailability-retriable-calls branch from a326352 to 19f04a9 Compare August 24, 2022 09:42
return body, true, nil
}

func isRemoteUnavailableStatusCode(statusCode int) bool {
Copy link
Member

Choose a reason for hiding this comment

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

теста похоже нет, проверить это надо будет

@AleksandrMatsko
Copy link
Member

Refactored and implemented in #1085

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants