RequeueFilter and () => next.status == Status.Open

[ailn]

There's a todo in code (https://github.com/twitter/finagle/blob/4863f23fbd5df339df159327d96fd05cac1023a5/finagle-core/src/main/scala/com/twitter/finagle/service/Retries.scala#L243)

This is only applied to RequeueFilter but not to RetryFilter. Is it valid at all?

If I use Mux then ThresholdFailureDetector will mark ClientSession as Closed after ping times out (5 seconds by default). Hence, RequeueFilter will stop retrying.

I might be missing something but when I pass precondition to always true it works fine and not affected by ThresholdFailureDetector.

My point is in case of service downtime RequeueFilter might stop retrying before exhausting budget because ThresholdFailureDetector marked ClientSession as Closed.

If there's a problem when the stack returns non-restartable service why similar logic is not implemented for RetryFilter?

[ljluestc]

// finagle/finagle-core/src/main/scala/com/twitter/finagle/service/Retries.scala

class RequeueFilter[Req, Rep](...) {
  ...
  private[this] val shouldRetry: PartialFunction[(ReqRepT, Try[Rep]), Boolean] = {
    case (_, Throw(f)) => RetryPolicy.Default.requeueable(f)
    case _ => false
  }

  // Updated precondition to always true
  private[this] val precondition: () => Boolean = () => true

  def apply(req: Req, service: Service[Req, Rep]): Future[Rep] = {
    if (!precondition()) Future.exception(new Exception("Precondition failed"))
    else {
      // Logic for requeuing retries
      ...
    }
  }
}

class RetryFilter[Req, Rep](...) {
  ...
  private[this] val shouldRetry: PartialFunction[(ReqRepT, Try[Rep]), Boolean] = {
    case (_, Throw(f)) => RetryPolicy.Default.shouldRetry(f)
    case _ => false
  }

  // Updated precondition to always true
  private[this] val precondition: () => Boolean = () => true

  def apply(req: Req, service: Service[Req, Rep]): Future[Rep] = {
    if (!precondition()) Future.exception(new Exception("Precondition failed"))
    else {
      // Logic for retrying
      ...
    }
  }
}