Skip to content

Commit 0a624ea

Browse files
committed
fix: restrict DispatchingTransport retries to transient failures only
sendAsync retried on any non-null error, wasting the retry budget on permanent 4xx failures. The exception hierarchy made this subtle: ResponseException and TransportException both extend IOException, so a naive instanceof-IOException check would retry HTTP 400/401/403/404/405 (delivered as ElasticsearchException via the LLRC ignore list) and HTTP 409 Conflict (delivered as ResponseException). Add isRetryable(Throwable): retry genuine transport failures (IOException subclasses that are neither ResponseException nor TransportException), HTTP 429 and HTTP 5xx (transient server pressure); do not retry ElasticsearchException (permanent app-level 4xx), TransportException (infrastructure misconfiguration), or ResponseException for other 4xx. Residual note: retrying IOException on bulk requests with use.autogenerated.ids=true can still create duplicate documents if ES applied operations before the connection dropped. This cannot be resolved inside DispatchingTransport without external versioning or ES-side deduplication.
1 parent 4c357d9 commit 0a624ea

1 file changed

Lines changed: 27 additions & 4 deletions

File tree

src/main/java/io/confluent/connect/elasticsearch/ElasticsearchClient.java

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,13 @@
2525
import co.elastic.clients.elasticsearch.indices.get_mapping.IndexMappingRecord;
2626
import co.elastic.clients.json.JsonpMapper;
2727
import co.elastic.clients.json.jackson.JacksonJsonpMapper;
28+
import co.elastic.clients.elasticsearch._types.ElasticsearchException;
2829
import co.elastic.clients.transport.ElasticsearchTransport;
2930
import co.elastic.clients.transport.Endpoint;
31+
import co.elastic.clients.transport.TransportException;
3032
import co.elastic.clients.transport.TransportOptions;
3133
import co.elastic.clients.transport.rest_client.RestClientTransport;
34+
import org.elasticsearch.client.ResponseException;
3235
import com.fasterxml.jackson.core.JsonProcessingException;
3336
import com.fasterxml.jackson.databind.ObjectMapper;
3437

@@ -760,10 +763,7 @@ private <RequestT, ResponseT, ErrorT> void sendAsync(
760763
delegate.performRequestAsync(request, endpoint, options)
761764
.whenCompleteAsync(
762765
(resp, err) -> {
763-
// Retry any transport failure (SocketTimeoutException, ConnectException,
764-
// ResponseException for 4xx/5xx, etc.), mirroring callWithRetries() in the
765-
// original HLRC buildConsumer() which caught Exception unconditionally.
766-
if (err != null && retriesLeft > 0) {
766+
if (err != null && retriesLeft > 0 && isRetryable(err)) {
767767
scheduler.schedule(
768768
() -> sendAsync(request, endpoint, options, result,
769769
retriesLeft - 1, backoffMs * 2),
@@ -778,6 +778,29 @@ private <RequestT, ResponseT, ErrorT> void sendAsync(
778778
);
779779
}
780780

781+
// ResponseException and TransportException are both IOException subclasses; they must be
782+
// checked before the general IOException branch so the more-specific type wins.
783+
private static boolean isRetryable(Throwable err) {
784+
// HTTP-level error from the LLRC: only retry transient codes
785+
if (err instanceof ResponseException) {
786+
int status = ((ResponseException) err).getResponse().getStatusLine().getStatusCode();
787+
return status == 429 || status >= 500;
788+
}
789+
// TransportException covers permanent structural failures: missing/invalid X-Elastic-Product
790+
// header, wrong content-type, missing response body, undecodable response, unrecognised
791+
// status code. None of these improve on retry.
792+
if (err instanceof TransportException) {
793+
return false;
794+
}
795+
// Permanent application-level ES errors (4xx via LLRC ignore list, deserialized by
796+
// ElasticsearchTransportBase as ElasticsearchException)
797+
if (err instanceof ElasticsearchException) {
798+
return false;
799+
}
800+
// Remaining IOExceptions are genuine transport failures (SocketTimeout, ConnectException…)
801+
return err instanceof IOException;
802+
}
803+
781804
@Override
782805
public JsonpMapper jsonpMapper() {
783806
return delegate.jsonpMapper();

0 commit comments

Comments
 (0)