[bharathgunapati]

Purpose of the change

https://issues.apache.org/jira/browse/FLINK-40278

Define, document, and test HTTP sink retry semantics for batch submission mode. The retry behaviour itself was implemented in FLINK-40277 (#54); this PR makes it explicit and locks it in with tests. Sink runtime code is unchanged.

In batch submission mode, each HTTP request carries several records and gets one response status:

• A retryable response retries all original entries of that HTTP request.
• A successful or ignored response completes all entries of that HTTP request.
• A fatal response, or a retryable response after the last retry, fails all entries of that HTTP request.

Partial per-record retry is out of scope; it would need a response processor that can parse target-specific per-record results.

Brief change log

• Add a "Retries and handling errors (Sink)" section to the Table and DataStream docs (en + zh), linked from each table of contents. It states that retries are disabled by default (http.sink.max-retries = 0), which failures are retried, that with the legacy http.sink.error.code properties no status code is retried (only requests without a response), and that one flush split into several HTTP requests is handled per request.
• Add JavaNetSinkHttpClientTest cases that run through the real PerRequestSubmitter and BatchRequestSubmitter:
  • per-request mode retries only the failed entry;
  • batch mode retries all entries of a retryable batch and then succeeds;
  • batch mode returns all entries as retryable once retries are exhausted;
  • batch mode fails all entries of a fatal batch without retrying;
  • batch mode completes all entries of an ignored batch;
  • a flush split by http.sink.request.batch.size retries only the records of the failed HTTP request;
  • retried records are regrouped into new HTTP requests (two failed POST requests separated by a PUT are resent as one request).
• The tests assert that retries receive the same HttpSinkRequestEntry instances (identity, not equality), so retries reuse the original entries rather than reconstructed request bytes. The split-flush tests record the entries of every HTTP request through the post-request callback.
• Test clients are closed in an @AfterEach so a failed assertion does not leak executors.

Verifying this change

mvn -pl flink-connector-http -DskipITs -Dskip.archunit=true -Dtest=JavaNetSinkHttpClientTest,HttpSinkClientWithRetryTest,BatchRequestSubmitterTest,HttpSinkWriterTest test (39 tests, all pass)

mvn -pl flink-connector-http spotless:check checkstyle:check

Mutation check: making BatchRequestSubmitter wrap each entry in a new HttpSinkRequestEntry fails the four batch identity tests, so the identity assertions are effective.

Impact and documentation

• Dependencies: no
• Public API: no
• Runtime per-record code paths: no
• Documentation: sink retry and batch failure semantics documented on the Table and DataStream pages (en + zh).

---

Was generative AI tooling used to co-author this PR?

• Yes (please specify the tool below)

Generated-by: Cursor (auto mode) for the initial change; Claude Code (Claude Fable 5.1) for the review follow-ups

[davidradl]

in general - if we retry the whole batch is it possible for some of them to be accepted? So retrying would produce duplicates - can we know which event(s) needs to be retried?

[davidradl]

can we get more granular information?

[MartijnVisser]

Not today: the sink only reads the status code. Per-record results need a parser for the endpoint's response body, out of scope for this ticket. A partly applied batch is resent in full.

[bharathgunapati]

Confirming what Martijn said: the sink classifies a response from its status code only, so it cannot tell which records of a batch the server applied. Per-record retry needs a response-body parser for the target, which is out of scope here. The docs state the limitation and recommend an idempotent endpoint for that case.

[davidradl]

have you manually tested this?

[MartijnVisser]

Checked locally with http.sink.request.batch.size 2 and three records: only the failed request's records are resent. Please add a test, nothing in the PR covers this yet.

[bharathgunapati]

@davidradl yes, both with unit tests and end to end.

@MartijnVisser added two tests in JavaNetSinkHttpClientTest that go through the real BatchRequestSubmitter:

• batchModeRetriesOnlyTheFailedRequestWhenFlushIsSplit: batch size 2, three records. The HTTP requests are [1,2] -> 500, [3] -> 200, then only [1,2] is resent.
• batchModeRegroupsRetriedEntriesIntoNewHttpRequests: POST, PUT, POST. The two failed POST records come back as one request on retry.

Both record the entries of every HTTP request through the post-request callback and compare by identity.

The same scenarios also run end to end in a Flink mini-cluster against an embedded HTTP server in bharathgunapati/flink-http-sink-examples (DataStreamHttpSinkIT, commit 699e9ee). Server log:

POST /split-batch -> 500 body=[{"id":1},{"id":2}]
POST /split-batch -> 200 body=[{"id":3}]
POST /split-batch -> 200 body=[{"id":1},{"id":2}]

POST /regroup -> 500 body=[{"id":1}]
PUT  /regroup -> 200 body=[{"id":2,"method":"PUT"}]
POST /regroup -> 500 body=[{"id":3}]
POST /regroup -> 200 body=[{"id":1},{"id":3}]

[MartijnVisser]

Thanks for the PR. The new batch tests fail when BatchRequestSubmitter rebuilds the entries, so the identity checks work. Can you add the AI disclosure from the PR template (FLINK-40758)?

[MartijnVisser]

With the legacy http.sink.error.code properties, no status code is retried, only failures without a response. I think that needs a sentence here, on all four pages.

[bharathgunapati]

Good catch. Added a paragraph on all four pages: with the legacy http.sink.error.code / http.sink.error.code.exclude properties no status code is retried, a code in the error list fails the job, every other code is success, and only requests that fail before a response is retried. That matches HttpSinkResponseClassifier, which returns only SUCCESS or FATAL_FAILURE on the legacy path.

[bharathgunapati]

Thanks for the reviews. Addressed in the latest push:

• Legacy error-code behaviour documented on all four pages.
• Two new tests for a flush split into several HTTP requests (batch size and method change), see the thread above for the end-to-end log.
• Test clients are now closed in an @AfterEach.
• AI disclosure added to the description per the new template.

@davidradl on the duplicates question: yes, if the server applied part of a batch before answering 500, the retry resends the whole batch. The sink cannot know which records were applied from a status code alone, so the docs ask for an idempotent endpoint when that matters.

The CI run for the new head is waiting for workflow approval. Could one of you approve it?

[MartijnVisser]

Thanks, the split-flush tests and the legacy error-code paragraph cover what I asked. Please replace the Co-Authored-By trailer with the Generated-by: line from the PR description, as the contribution guide asks.

[bharathgunapati]

Done. The commit now ends with the Generated-by line from the description instead of the Co-Authored-By trailer (head b4e74c7, no content change). I applied the same to #58, which had the identical trailer.

[MartijnVisser]

LGTM % CI passing