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

Implement Retry functionality #4109

Open
wants to merge 5 commits into
base: series/3.x
Choose a base branch
from
Open

Conversation

iRevive
Copy link
Contributor

@iRevive iRevive commented Jul 26, 2024

The PR is influenced by #3135.

In my opinion, Retry must carry the error type. That way, we can implement retry functionality on top of the Handle from the cats-mtl.

I also decided to keep the name (perhaps the 'description' fits better?) around. That way, toString provides enough details to understand the retry strategy:

val policy = Retry
  .exponentialBackoff[IO, Throwable](1.second)
  .withCappedDelay(2.seconds)
  .withMaxRetries(10)
  
println(policy)
// MaxRetries(CapDelay(Backoff(baseDelay=1 second, multiplier=Const(2.0), randomizationFactor=0.5), cap=2 seconds), max=5)

Usage

Retry on all errors

val policy = Retry
  .exponentialBackoff[IO, Throwable](1.second)
  .withMaxRetries(10)

// retries 10 times at most using an exponential backoff strategy
IO.raiseError(new RuntimeException("oops")).retry(policy)

Retry on some errors (e.g. TimeoutException)

val policy = Retry
  .exponentialBackoff[IO, Throwable](1.second)
  .withMaxRetries(10)
  .withErrorMatcher(Retry.ErrorMatcher[IO, Throwable].only[TimeoutException])

// retries 10 times at most using an exponential backoff strategy
IO.raiseError(new TimeoutException("timeout")).retry(policy)

// gives up immediately
IO.raiseError(new RuntimeException("oops")).retry(policy)

Retry on all errors except the TimeoutException

val policy = Retry
  .exponentialBackoff[IO, Throwable](1.second)
  .withMaxRetries(10)
  .withErrorMatcher(Retry.ErrorMatcher[IO, Throwable].except[TimeoutException])

// retries 10 times at most using an exponential backoff strategy
IO.raiseError(new RuntimeException("oops")).retry(policy)

// gives up immediately
IO.raiseError(new TimeoutException("timeout")).retry(policy)

A few points to discuss:

  1. A confusion between withMaxDelay, withCappedDelay, withMaxCumulativeDelay. Even though I provided the documentation with examples, these three methods are confusing. Can we find better names?
  2. Is my implementation of the exponential backoff correct?

@iRevive iRevive force-pushed the retry branch 2 times, most recently from de14d85 to c7abc4e Compare July 26, 2024 17:41
Comment on lines +566 to +715
private def doRetry[F[_], A, E](
retry: Retry[F, E],
onRetry: Option[(Status, E, Decision) => F[Unit]]
)(fa: F[A])(implicit F: GenTemporal[F, E]): F[A] = {
def onError(status: Status, error: E): F[Either[Status, A]] =
retry
.decide(status, error)
.flatTap(decision => onRetry.traverse_(_(status, error, decision)))
.flatMap {
case retry: Decision.Retry =>
F.delayBy(F.pure(Left(status.withRetry(retry.delay))), retry.delay)
case _: Decision.GiveUp =>
F.raiseError(error)
}

F.tailRecM(Status.initial) { status =>
fa.redeemWith(error => onError(status, error), success => F.pure(Right(success)))
}
}
Copy link
Contributor Author

@iRevive iRevive Jul 26, 2024

Choose a reason for hiding this comment

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

The evaluation of the retry strategy.

@iRevive iRevive force-pushed the retry branch 5 times, most recently from 3cbaeff to 8af8301 Compare July 28, 2024 09:06
Comment on lines 521 to 553
"retry only on matching errors" in ticked { implicit ticker =>
type F[A] = EitherT[IO, Errors, A]

val maxRetries = 2
val delay = 1.second

val error = new Error1
val policy = Retry
.constantDelay[F, Errors](delay)
.withMaxRetries(maxRetries)
.withErrorMatcher(Retry.ErrorMatcher[F, Errors].only[Error1])

val expected: List[RetryAttempt] = List(
(Status(0, Duration.Zero), Decision.retry(delay), error),
(Status(1, 1.second), Decision.retry(delay), error),
(Status(2, 2.seconds), Decision.giveUp, error)
)

val io: F[Unit] = Handle[F, Errors].raise[Errors, Unit](error)

val run =
for {
ref <- IO.ref(List.empty[RetryAttempt])
result <- mtlRetry[F, Errors, Unit](
io,
policy,
(s, e: Errors, d) => EitherT.liftF(ref.update(_ :+ (s, d, e)))
).value
attempts <- ref.get
} yield (result, attempts)

run must completeAs((Left(error), expected))
}
Copy link
Contributor Author

@iRevive iRevive Jul 28, 2024

Choose a reason for hiding this comment

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

Retry works with MTL too.

@iRevive iRevive force-pushed the retry branch 2 times, most recently from e60f16c to 7f25d62 Compare July 28, 2024 09:28
@TobiasRoland
Copy link

I'd really love for this to land 🚀 - but I see there's been no comments for 4 months - @iRevive is this waiting on feedback or approvals, or is discussion ongoing somewhere off Github?

@iRevive
Copy link
Contributor Author

iRevive commented Nov 5, 2024

I'd really love for this to land 🚀 - but I see there's been no comments for 4 months - @iRevive is this waiting on feedback or approvals, or is discussion ongoing somewhere off Github?

I'm still waiting for the feedback.

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.

3 participants