8000 [Utils] Create exponential backoff utility class by gregfurman · Pull Request #12264 · localstack/localstack · GitHub
[go: up one dir, main page]
Skip to content

[Utils] Create exponential backoff utility class #12264

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
Merged
merged 3 commits into from
Feb 18, 2025
Merged

[Utils] Create exponential backoff utility class #12264

merged 3 commits into from
Feb 18, 2025

Conversation

gregfurman
Copy link
Contributor
@gregfurman gregfurman commented Feb 13, 2025 •
edited
Loading

Motivation

This PR creates a re-usable component for calculating backoff durations. This was inspired by the exponential backoff algorithm from Google's HTTP Client Library for Java.

Changes

  • Adds an ExponentialBackoff utility class whose next_backoff method calculates and returns the next duration that should be waited for.

Usage

The below shows this class being used with a maximum of 3 attempts (with no random sampling between intervals):

import time

from localstack.utils.backoff import ExponentialBackoff

boff = ExponentialBackoff(max_retries=3, randomization_factor=0)

time.wait(boff.next_backoff()) # waits 0.5s
time.wait(boff.next_backoff()) # waits 0.75s
time.wait(boff.next_backoff()) # waits 0.125s

# immediately returns since max_retries have been met
time.wait(boff.next_backoff())

# reset the backoff to the initial state
boff.reset()

# begin backing off again
time.wait(boff.next_backoff()) # waits 0.5s
# ...

@gregfurman gregfurman added the area: performance Make LocalStack go rocket-fast label Feb 13, 2025
@gregfurman gregfurman added this to the Playground milestone Feb 13, 2025
@gregfurman gregfurman self-assigned this Feb 13, 2025
@gregfurman gregfurman added the semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases label Feb 13, 2025
@gregfurman gregfurman changed the title [util] Create exponential backoff utility class [Utils] Create exponential backoff utility class Feb 13, 2025
@gregfurman gregfurman force-pushed the add/util/retry-backoff branch from 06bdefc to 11b5478 Compare February 13, 2025 15:33
@gregfurman gregfurman force-pushed the add/util/retry-backoff branch from 11b5478 to 6a8b01f Compare February 13, 2025 15:36
@gregfurman gregfurman marked this pull request as ready for review February 13, 2025 15:36
@gregfurman gregfurman requested a review from tiurin February 13, 2025 15:53
@github-actions GitHub Actions
Copy link
github-actions bot commented Feb 13, 2025 •
edited
Loading

LocalStack Community integration with Pro

    2 files  ±0      2 suites  ±0   1h 50m 24s ⏱️ - 2m 14s
4 099 tests +2  3 767 ✅ +1  332 💤 +1  0 ❌ ±0 
4 101 runs  +2  3 767 ✅ +1  334 💤 +1  0 ❌ ±0 

Results for commit f17c9eb. ± Comparison against base commit a79d200.

This pull request skips 1 test. 
tests.aws.services.events.test_events_schedule.TestScheduleRate ‑ tests_schedule_rate_target_sqs

♻️ This comment has been updated with latest results.

tiurin
tiurin reviewed Feb 17, 2025
View reviewed changes
Copy link
Contributor
@tiurin tiurin left a comment

Choose a reason for hiding this comment

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

Nice implementation!

Utility in itself is easy to understand, it would be great to see a companion example PR with its real-world usage, to understand the reason for this addition and how it fits the solution (e.g. to understand why we write utility ourselves and don't use some ready solution).

@gregfurman gregfurman mentioned this pull request Feb 18, 2025
Merged
@gregfurman
Copy link
Contributor Author
gregfurman commented Feb 18, 2025 •
edited
Loading

@tiurin Thanks for the review!

[...] it would be great to see a companion example PR with its real-world usage, to understand the reason for this addition and how it fits the solution

I've linked a companion PR showing its usage 🙂

(e.g. to understand why we write utility ourselves and don't use some ready solution).

From what I've read, most large + well maintained external retry libraries provide us with retry function decorators (see https://tenacity.readthedocs.io/en/latest/ or https://github.com/litl/backoff) which would be far more difficult to integrate into our current implementations than the class-based approach.

@tiurin
Copy link
Contributor
tiurin commented Feb 18, 2025

I've linked a companion PR showing its usage 🙂

Thank you! ❤️

From what I've read, most large + well maintained external retry libraries provide us with retry function decorators (see https://tenacity.readthedocs.io/en/latest/ or https://github.com/litl/backoff) which would be far more difficult to integrate into our current implementations than the class-based approach.

Indeed, in linked PR case of a loop with a complex exit condition a separate backoff mechanism is better suited! 👍 It's not too easy to wrap something with a simple retry there, nor needed probably.
Also, for backoff one the maintainer seems to be inactive, so importing it might not be the best solution, so tenacity looks like the main candidate, and indeed it is not well suited.

For the reference, we also have our own collection of retry/wait functions here (used mostly in tests but also in some ext production code as well), with wait_until having a notion of exponential backoff:

localstack/localstack-core/localstack/utils/sync.py

Line 16 in 958890a

def wait_until(

I do agree a separate class that you suggest is easier to adjust to different use cases as its only job is to keep track of numbers. 👍

@gregfurman
Copy link 8000
Contributor Author
gregfurman commented Feb 18, 2025 •
edited
Loading

For the reference, we also have our own collection of retry/wait functions here (used mostly in tests but also in some ext production code as well), with wait_until having a notion of exponential backoff:

Good reference here. However... those functions wont allow us much flexability and could cause blocking waits + shutdowns since the wait_until (or equivalent) doesn't provide us the ability to cancel early.

Also, I prefer the utility-based approach since it makes no assumptions about where the backoff is going to be called from.

That is, the boff object exists purely to provide some caller with what backoff should be after a certain number of invocations as opposed to actually waiting on that calculated duration.

@gregfurman gregfurman requested a review from tiurin February 18, 2025 14:13
tiurin
tiurin approved these changes Feb 18, 2025
View reviewed changes
Copy link
Contributor
@tiurin tiurin left a comment

Choose a reason for hiding this comment

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

Great work @gregfurman! I just have one last validation comment, leaving it up to you to address in this or in comanion PR as you see fit. 👍

@gregfurman gregfurman merged commit 9bc5bc9 into master Feb 18, 2025
31 checks passed
@gregfurman gregfurman deleted the add/util/retry-backoff branch February 18, 2025 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tiurin tiurin tiurin approved these changes

@joe4dev joe4dev Awaiting requested review from joe4dev

@dfangl dfangl Awaiting requested review from dfangl

@bentsku bentsku Awaiting requested review from bentsku

Assignees

@gregfurman gregfurman

Labels
area: performance Make LocalStack go rocket-fast semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases
Projects
None yet
Milestone
Playground
Development

Successfully merging this pull request may close these issues.
2 participants
@gregfurman @tiurin
0