search

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

finagle-core: Deposit budget once in MethodBuilder #957

Closed DieBauer closed 10 months ago

DieBauer commented 12 months ago

Problem

The methodbuilder interface applies the same retrybudget for RetryFilter and RequeueFilter in the same stack. Resulting in double deposit of retrybudget.

Solution

Re-use the WithdrawOnlyRetryBudget to prevent depositing successes in the RequeueFilter and only deposit in the RetryFilter. Same solution as the Retries.moduleWithRetryPolicy takes.

Result

Retrybudget only gets deposited once for a request.

Closes #956

mattdickinson5 commented 10 months ago

@DieBauer - this code isn't compiling, and when i fix it the MethodBuilderRetryTest test does not pass.

DieBauer commented 10 months ago

I've rebased to develop (SNAPSHOTS were shifted). And an assert was indeed wrong. In the last testcase .retry.disabled, you do not expect any 'retries' stat to be present!

Let me know what you think!