RetryFilter may retain request payload till response
- Dominant language
- Java
- Stars
- 1k
- Forks
- 229
- Avg merge
- 23h 23m
- Merged PRs (30d)
- 16
Description
Originating discussion:
https://github.com/servicetalk/servicetalk/pull/261#discussion_r250881023
Since, `retryFor` predicate requires `HttpRequestMetadata` and we use this predicate in the response path, we retain this object longer than otherwise (request will be eligible for GC after it has been written). If this is just metadata, it isn't that bad but it may be that the user converted the client to an aggregated client and the `HttpRequestMetadata` received by the retry filter contains the entire payload as a realized `Publisher`.
The motivation for this change were two fold:
1. Allow for always retry `RetryableException`.
2. Provide the metadata in the `retryFor` predicate to make a retry decision based on the request.
In order to address 1. we can always add retry operator which at the minimum retries `RetryableException`.
For addressing 2. only for cases when such a feature is required, we can optionally take a `retryFor` provider that will be called for each `request` and can provide the predicate given the request. Such a provider can capture the request if required.
Contributor guide
Research direction
Start by tracing RetryFilter's retryFor predicate through the request and response paths, focusing on how HttpRequestMetadata and RetryableException are retained. Compare the proposed always-retry behavior with an optional request-based retryFor provider; done means retry decisions remain possible without retaining an aggregated request payload until the response.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- networking
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100