HotSushi opened a new pull request, #17947:
URL: https://github.com/apache/iceberg/pull/17947
## What & why
The REST client's `ExponentialHttpRequestRetryStrategy` only applies the
idempotent-retriable status codes (408, 500, 502, 503, 504) when the HTTP
method itself is idempotent (`Method.isIdempotent`). All catalog mutation
endpoints are `POST`, for which `Method.isIdempotent` is `false`, so a mutation
that already carries an `Idempotency-Key` header — which the server guarantees
is safe to replay — was not retried on `502`/`504`, or on `503` without a
`Retry-After` header.
This completes the client side of the idempotency support added in #14740
(spec: #14196): the client already generated and sent the key, but its own
retry logic ignored it. This change lets the key the client sends actually
authorize a retry.
## Behavior change
| Response on a POST mutation carrying an Idempotency-Key | Safe to retry? |
Before | After |
|---|---|---|---|
| 502 / 504 | Yes | does not retry | retries |
| 503 without `Retry-After` | Yes | does not retry | retries |
| 503 with `Retry-After` | Yes | retries | retries (unchanged) |
Requests **without** an `Idempotency-Key` keep the previous conservative
behavior — the new path only opens up when the safety contract is in place (the
client attaches a key only when the server advertised
`idempotency-key-lifetime` in the config response).
## Changes
- `ExponentialHttpRequestRetryStrategy`: treat a request carrying the
`Idempotency-Key` header as retry-safe for the idempotent-retriable codes, in
both the response path (`shouldRetryIdempotent`) and the network-exception path
(`retryRequest(HttpRequest, IOException, …)`).
- Added `TestExponentialHttpRequestRetryStrategy` cases: a keyed POST
retries on {429, 503, 500, 502, 504, 408}; an un-keyed POST does not retry on
{503, 500, 502, 504, 408}.
No public API changes (`ExponentialHttpRequestRetryStrategy` is
package-private). This is a client-only change and does not modify
`open-api/rest-catalog*`.
## Testing
`./gradlew :iceberg-core:test --tests
"org.apache.iceberg.rest.TestExponentialHttpRequestRetryStrategy"` — passing.
## Note on scope
Beyond the response-code cases above, this also extends the
**network-exception** retry path to honor the header, so a dropped connection
on a keyed POST is retried for the same reason (the server dedupes on replay).
This is a deliberate extension for symmetry; happy to split it into a follow-up
if reviewers prefer to keep this PR to the response-code path only.
## AI assistance
Drafted with AI assistance (code and test scaffolding). The logic was
reviewed by the author and verified against the existing retry-strategy tests;
the network-exception path extension noted above is the main area worth
reviewer attention.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
