This PR allows to define expected behaviour by settingsink.delivery-guarantee: eithernone (no retries, this is effectively the same as at-most-once) orat-least-once.exactly-once is not supported.

Implemented based on aws connectors usingAsyncSinkBase, 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-L223

PR Checklist

Tests added
Changelog updated

Grzegorz Kołakowski added3 commits

August 28, 2024 14:27

Enable HTTP Sink request retries

1181334

Add tests

c68fca9

Add RateLimitingStrategy

0729db7

davidradl reviewed

Aug 30, 2024

View reviewed changes

src/main/java/com/getindata/connectors/http/internal/SinkHttpClientResponse.java

		.collect(Collectors.toList());
		}

		publicList<HttpRequest>getFailedRequests() {

Copy link

Collaborator

davidradlAug 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

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).

Copy link

MemberAuthor

grzegorz8Aug 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

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 :)

MarekMaj reviewed

Sep 2, 2024

View reviewed changes

src/main/java/com/getindata/connectors/http/internal/sink/HttpSinkWriter.java

		// 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 =newArrayList<>();

Copy link

Contributor

MarekMajSep 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Can we have multiple values for the same key in one list ofrequestEntries? 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?