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

Add a schedule-aware repeat #57

Closed
rucek opened this issue Dec 4, 2023 · 8 comments · Fixed by #172
Closed

Add a schedule-aware repeat #57

rucek opened this issue Dec 4, 2023 · 8 comments · Fixed by #172
Assignees
Labels
enhancement New feature or request retries

Comments

@rucek
Copy link
Contributor

rucek commented Dec 4, 2023

While the retry mechanism allows for repeating an operation - according to a Schedule in case of error, we'd also like to have a schedule-aware repeat, which would run an operation according to a Schedule or perhaps until a stop condition is met.

Some things to consider:

  • what happens if the operation fails before the scheduled number of repetitions? Maybe this could be expressed with a multi-policy (see: Add multi-policies for retries #55) that combines repeat and retry?
  • perhaps the stop condition could be thought of yet another Schedule like UntilResult[T](p: T => Boolean) or Until(p: => Boolean), or both?
@rucek rucek added enhancement New feature or request retries labels Dec 4, 2023
@adamw
Copy link
Member

adamw commented Apr 18, 2024

Some operations that we'd like to have:

  • schedule at fixed rate
  • schedule with an initial delay

Should we measure time between start of each operation (start2-start1), or the pause between operation invocations (start2-end1)? Or maybe this should be configurable?

@micossow
Copy link
Contributor

micossow commented Jul 3, 2024

I did some API prototypic which led to few observations:

  1. We could refactor current retry* functions to schedule* and reimplement retry* on top of them, specifically:
def schedule[T](
    schedule: Schedule,
    onTick: (Int, Either[Throwable, T]) => Unit = (_: Int, _: Either[Throwable, T]) => (),
    shouldRetryOnError: Throwable => Boolean = (_: Throwable) => false,
    shouldContinue: T => Boolean = (_: T) => true
)(operation: => T): T =
  scheduleEither(schedule, onTick, shouldRetryOnError, shouldContinue)(Try(operation).toEither).fold(throw _, identity)

def scheduleEither[E, T](
    schedule: Schedule,
    onTick: (Int, Either[E, T]) => Unit = (_: Int, _: Either[E, T]) => (),
    shouldRetryOnError: E => Boolean = (_: E) => false,
    shouldContinue: T => Boolean = (_: T) => true
)(operation: => Either[E, T]): Either[E, T] =
  scheduleWithErrorMode(EitherMode[E])(schedule, onTick, shouldRetryOnError, shouldContinue)(operation)

def scheduleWithErrorMode[E, F[_], T](em: ErrorMode[E, F])(
    schedule: Schedule,
    onTick: (Int, Either[E, T]) => Unit = (_: Int, _: Either[E, T]) => (),
    shouldRetryOnError: E => Boolean = (_: E) => false,
    shouldContinue: T => Boolean = (_: T) => true
)(operation: => F[T]): F[T] = ...

Then retryWithErrorMode could reuse scheduleWithErrorMode like this:

def retryWithErrorMode[E, F[_], T](em: ErrorMode[E, F])(policy: RetryPolicy[E, T])(operation: => F[T]): F[T] =
  scheduleWithErrorMode(em)(
    policy.schedule,
    policy.onRetry,
    policy.resultPolicy.isWorthRetrying,
    t => !policy.resultPolicy.isSuccess(t)
  )(operation)
  1. The language used in Schedule docs/symbol names couples it to the retry concept, so that would need to be refactored. For example maxAttempts instead of maxRetries
  2. Then Schedule trait can be extended with additional variants like FixedRate, initial delay, etc. The problem is the additional variants wouldn't make sense for retry as well as some of the existing variants, like Backoff wouldn't make sense for scheduling, so I'm not sure about it.

See #172

@adamw
Copy link
Member

adamw commented Jul 4, 2024

I still think repeat and retry are in their essence the same thing. You're right we might need to change the vocabulary a bit, but both of these perform the same basic steps:

  1. run a computation
  2. inspect the result, and either:
    3a. sleep and restart
    3b. finish

The difference is that for repeat the inspection in step 2. checks if the result is successful - and if so, continues, otherwise finishes with an error. For retry, it's the opposite: it checks if the result is an error, and if so continues, otherwise finishes with success.

So with the definition of "success" and "error" being user-provided (in the form of a function: Either[successValue, errorValue] => shouldContinue) it seems that retry and repeat are both specialisations of this runWithSchedule function (or however we call it). Maybe even retry is a specialisation of repeat?

I think the Backoff also makes sense for repeat, just the naming might need to be different - maybe simply Exponential? You might repeat something with exponentially increasing pauses, why not?

FixedRate is really what we currently call Delay.forever, no? We just might need two variants of this, one which would measure the delay between end-of-previous and start-of-next, and another one which would measure the delay between start-of-previous and start-of-next (that is, disregarding the time it takes to actually run the computation). But that would be useful for retries as well.

What we could have is use-case-specific DSLs for retries & repeats, using retry or repeat-specific vocabulary for naming the constructor methods, but under the covers yield Schedule instances.

Another thing to consider: it might be useful to linearly combine schedules. Each schedule would have to be combined with the number of repetitions it handles (0, 1, 2, ... infinity). Then, we might want to say: run schedule1, followed by schedule2, etc. The schedules would be applied basing on the current repetition number, as long as they are available. Then we could express things like "initial delay" (handles 0 repetitions, but includes a delay), or "retry 3 times, then with backoff indefinitely".

@micossow
Copy link
Contributor

micossow commented Jul 4, 2024

FixedRate is really what we currently call Delay.forever, no? We just might need two variants of this, one which would measure the delay between end-of-previous and start-of-next, and another one which would measure the delay between start-of-previous and start-of-next (that is, disregarding the time it takes to actually run the computation). But that would be useful for retries as well.

I wanted to distinguish these two options by putting them into separate Schedule variants, so that:

  • Delay remains as it was originally (sleep after previous operation ends)
  • FixedRate implements the other option, where we sleep [interval - (now - start time of prev operation)] or immediately (when it's <= 0)

Also, for now I created runScheduled which is called by repeat and retry, but it can be "reduced" to retry using repeat, but I think the first approach is more clean.

@adamw
Copy link
Member

adamw commented Jul 4, 2024

I think both delay strategies can be useful in both situations (repeat/retry). So ideally we want to be able to use both.

@micossow
Copy link
Contributor

micossow commented Jul 5, 2024

By design all the strategies can be used in repeat and retry

@adamw
Copy link
Member

adamw commented Jul 5, 2024

Right, in case maybe it would make sense to parametrise all of Delay, Backoff/Exponential, FixedRate with how the time between subsequent attempts is measured - either end <-> start, or start <-> start?

@micossow
Copy link
Contributor

micossow commented Jul 5, 2024

  • removed Schedule.FixedRate
  • added DelayPolicy as a parameter of runScheduled (decoupled from Schedule)
enum DelayPolicy:
  case SinceTheStartOfTheLastInvocation, SinceTheEndOfTheLastInvocation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request retries
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants