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

RetryingReadableStore to support retrying on only certain exceptions #214

Open
tanin47 opened this issue Feb 7, 2014 · 4 comments
Open

Comments

@tanin47
Copy link

tanin47 commented Feb 7, 2014

I would like to only retry on some exceptions. For example, these exceptions:

  val DefaultRetryExceptions = Seq(
    classOf[GlobalRequestTimeoutException],
    classOf[ServiceTimeoutException],
    classOf[InterruptedException],
    classOf[NoBrokersAvailableException])

But I wouldn't want to retry on some other exceptions (e.g. application-level exceptions). Therefore, I need a way to specify it.

@rubanm
Copy link
Contributor

rubanm commented Feb 8, 2014

What do you think of passing in (shouldRetry : Exception => Boolean) instead?

@tanin47
Copy link
Author

tanin47 commented Feb 9, 2014

Good idea! I think that'd be more flexible for consumers.

@tanin47 tanin47 self-assigned this Mar 2, 2014
@tanin47
Copy link
Author

tanin47 commented Mar 2, 2014

I'm not sure if I should modify pred: Option[V] => Boolean to pred:Try[Option[V]] => Try[Option[V]]. In this way, a consumer can decide to ignore certain exceptions and retry on some exceptions.

It'll look like:

class RetryingReadableStore[-K, +V](store: ReadableStore[K, V], backoffs: Iterable[Duration])(pred: Try[Option[V]] => Try[Option[V]])(implicit timer: Timer) extends ReadableStore[K, V] {

  private[this] def getWithRetry(k: K, backoffs: Iterable[Duration]): Future[Option[V]] =
    store.get(k) transform { vOpt =>
      Future.const(pred(vOpt))
    } transform {
      case Return(t) => Future.value(t)
      case Throw(e) =>
        backoffs.headOption match {
          case None => FutureOps.retriesExhaustedFor(k)
          case Some(interval) => interval match {
            case Duration.Zero => getWithRetry(k, backoffs.tail)
            case Duration.Top => FutureOps.missingValueFor(k)
            case _ => Future.flatten {
              timer.doLater(interval) {
                getWithRetry(k, backoffs.tail)
              }
            }
          }
        }
    }

  override def get(k: K) = getWithRetry(k, backoffs)
}

Please let me know what you think

@rubanm
Copy link
Contributor

rubanm commented Mar 5, 2014

Not sure what pred should produce to denote an ignorable exception here? With the above code, seems like it would produce Return(optv) which doesn't say if this is the actual value to be returned to the consumer, or a retryable case?

An alternative can perhaps be Try[Option[V]] => Try[Either[Option[V], Unit]]. Left would mean predicate passed, Right would mean retry. I feel this bloats the predicate function though. Might be better to keep them separate if there's no simple way to combine.

@tanin47 tanin47 removed their assignment Mar 17, 2015
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

No branches or pull requests

2 participants