Enable HTTP Sink request retries #120
Conversation
| .collect(Collectors.toList()); | ||
| } | ||
|
|
||
| public List<HttpRequest> getFailedRequests() { |
There was a problem hiding this comment.
I did a google and found. https://stackoverflow.com/questions/47680711/which-http-errors-should-never-trigger-an-automatic-retry . It seems to be that you should only retry if the status code is retriable. Maybe group into successful, retriable and failed (no retry).
There was a problem hiding this comment.
Thanks! Good point. I may refactor the code so that there are 3 statuses as you suggested. The change may also affect lookup, not only sink. So as a "side-effect" I may introduce lookup retry feature :)
| // See com.getindata.connectors.http.internal.sink.httpclient. | ||
| // JavaNetSinkHttpClient#putRequests where requests are submitted sequentially and | ||
| // then their futures are joined sequentially too. | ||
| List<HttpSinkRequestEntry> failedRequestEntries = new ArrayList<>(); |
There was a problem hiding this comment.
Can we have multiple values for the same key in one list of requestEntries? It seems to me that by introducing retry mechanism we can interfere with final entries ordering. That may be the case, when latest key entry is saved successfully and then older is retried, effectively overwriting latest value. Do I read that correctly?
Description
Currently HTTP Sink does not retry failed requests which make it impossible to use in many scenarios.
This PR allows to define expected behaviour by setting
sink.delivery-guarantee: eithernone(no retries, this is effectively the same as at-most-once) orat-least-once.exactly-onceis not supported.Implemented based on aws connectors using
AsyncSinkBase, for example: https://github.com/apache/flink-connector-aws/blob/main/flink-connector-aws/flink-connector-aws-kinesis-streams/src/main/java/org/apache/flink/connector/kinesis/sink/KinesisStreamsSinkWriter.java#L213-L223PR Checklist