twitter / finagle

A fault tolerant, protocol-agnostic RPC system
https://twitter.github.io/finagle
Apache License 2.0
8.78k stars 1.45k forks source link

Methodbuilder retry budget gets double deposit #956

Closed DieBauer closed 10 months ago

DieBauer commented 12 months ago

Describe the bug The methodbuilder introduces a RetryFilter that gets a RetryBudget. This budget is passed to the stack and picked up by the RequeueFilter. On a successful response the Retry and RequeueFilter deposit both for the same request, effectively doubling the budget.

To Reproduce With such a budget

val budget = Retries.Budget(RetryBudget(ttl = 10.seconds, 10, 0.1))

do 5 calls using the method builder interface and see that the budget.retryBudget.balance == 101

expected is to reach 101 after 10 calls. (10% gets deposited).

  test("retries and requeues do not double deposit RetryBudget") {
    val stats = new InMemoryStatsReceiver()
    val budget = Retries.Budget(RetryBudget(ttl = 10.seconds, 10, 0.1))

    val svc = Service.mk[Int, Int] { _ =>
        Future.value(1)
    }

    val stack = Retries
      .moduleRequeueable[Int, Int]
      .toStack(Stack.leaf(Stack.Role("test"), ServiceFactory.const(svc)))
    val ps =
      Stack.Params.empty +
        param.Label(clientName) +
        param.Stats(stats) +
        budget
    val stackClient = TestStackClient(stack, ps)

    val methodBuilder = MethodBuilder.from("retry_it", stackClient)
    val client = methodBuilder
      .newService("client")

    assert(budget.retryBudget.balance == 100)
    (1 to 10).foreach(_ => Await.result(client(1), 5.seconds))
    assert(budget.retryBudget.balance == 101) // currently it's 102. 
  }

Expected behavior Deposit is only called once for a succesful request.

Additional context The RequeueFilter has a WithdrawOnlyBudget for this exact case, but that only works for the Client (Retries.moduleWithRetryPolicy) , methodbuilder is not aware of this configuration.

xin301x commented 12 months ago

这是来自QQ邮箱的假期自动回复邮件。   您好,我最近正在休假中,无法亲自回复您的邮件。我将在假期结束后,尽快给您回复。

mattdickinson5 commented 10 months ago

resolved in https://github.com/twitter/finagle/pull/957