featzhang commented on PR #33:
URL:
https://github.com/apache/flink-connector-http/pull/33#issuecomment-4404592972
Addressed the review comment — went with **Option 1** and aligned the sink
retry configuration with the lookup source. Latest commit: `82b9450`.
### What changed
**New sink options (mirroring `http.source.lookup.*`):**
| Option | Default | Description |
|---|---|---|
| `http.sink.retry-strategy.type` | `exponential-delay` | `fixed-delay` or
`exponential-delay` |
| `http.sink.retry-codes` | `500,503,504` | Status codes treated as
transient (retried) |
| `http.sink.success-codes` | `2XX` | Status codes treated as success |
| `http.sink.retry-strategy.fixed-delay.delay` | `1s` | Delay for
`fixed-delay` |
| `http.sink.retry-strategy.exponential-delay.initial-backoff` | `1s` |
Initial backoff for `exponential-delay` |
| `http.sink.retry-strategy.exponential-delay.max-backoff` | `1min` | Max
backoff cap |
| `http.sink.retry-strategy.exponential-delay.backoff-multiplier` | `2.0` |
Multiplier per attempt |
`http.sink.retry.times` keeps its original meaning (max retry attempts).
### Implementation notes
- Reused `HttpResponseChecker` + `HttpCodesParser` for the status-code
classification — same machinery the lookup source uses.
- Generalised `RetryConfigProvider` through a `RetryOptionKeys` bag so both
source and sink share the strategy logic (fixed-delay / exponential-delay with
max retries).
- Sink retries use resilience4j's `IntervalFunction`, same as the lookup
source, so behaviour is consistent.
- `SinkHttpClientResponse` now distinguishes **retryable** vs **fatal**
failures: only retryable ones are retried; fatal status codes count against
`numRecordsSendErrorsCounter` immediately and do not block the pipeline.
- The legacy `http.sink.error.code` / `http.sink.error.code.exclude` options
remain untouched for backwards compatibility — if neither `retry-codes` nor
`success-codes` is set, behaviour is unchanged.
- Both English and Chinese docs updated with the new options and a dedicated
*Retries and handling errors (Sink)* section.
### Tests
- New `SinkRetryConfigTest` covering: defaults, fixed-delay,
exponential-delay, ISO-8601 + Flink duration parsing, custom retry/success
codes, and error cases.
- New `HttpSinkWriterTest#testFatalFailuresAreNotRetried` asserting fatal
status codes are not retried even when `retry.times > 0`.
- All previously-passing sink tests still green:
- `HttpSinkWriterTest` (5), `SinkRetryConfigTest` (7),
`HttpDynamicTableSinkFactoryTest` (4), `JavaNetSinkHttpClientConnectionTest`
(13), `BatchRequestHttpDynamicSinkInsertTest` (13),
`PerRequestHttpDynamicSinkInsertTest` (5), `HttpPostRequestCallbackFactoryTest`
(3).
Ready for another look 🙏
--
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]
