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

Adding a more flexible retry function to the retryService #178535

Closed
maryam-saeidi opened this issue Mar 12, 2024 · 4 comments
Closed

Adding a more flexible retry function to the retryService #178535

maryam-saeidi opened this issue Mar 12, 2024 · 4 comments
Labels
Team:QA Team label for QA Team

Comments

@maryam-saeidi
Copy link
Member

maryam-saeidi commented Mar 12, 2024

Summary

In this PR, the security team (@jpdjere) added a retry utility that is very flexible and it accepts parameters such as:

  • timeout
  • retries
  • retryDelay

I copied this utility to our observability API integration tests, and I did some adjustment regarding logging as we don't need to have error logs in case of a failure attempt (I prefer the logging that we have in retryService instead.)

I think it would be good to have this code shared between different tests, either in a package or maybe moving it to the retryService itself.

The end goal is to have this functionality shared between tests and replace all the instances of pRetry with this utility instead: (Second item in the description of this PR)

  • In case of throwing an error in pRetry, when we have 10 retries, it does not log the retry attempts and we end up in the situation that is mentioned in this comment, item 3
    • This can be related to exponential backoff stated here and not logging the attempts until either timeout or successful try.

      The example below will retry a potentially failing dns.resolve operation 10 times using an exponential backoff strategy. With the default settings, this means the last attempt is made after 17 minutes and 3 seconds.

Before After
image image
@maryam-saeidi maryam-saeidi added the Team:QA Team label for QA Team label Mar 12, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/appex-qa (Team:QA)

@dmlemeshko
Copy link
Member

dmlemeshko commented Mar 13, 2024

I agree it is a useful improvement. I also think that having retry capabilities within single service is easy to support and re-use across different tests in the long run, opened #178660

@maryam-saeidi
Copy link
Member Author

@dmlemeshko Shall we close this ticket? I think you covered it in your PR, right?

@dmlemeshko
Copy link
Member

@dmlemeshko Shall we close this ticket? I think you covered it in your PR, right?

Yes, I think we can close it.

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

No branches or pull requests

3 participants