-
Notifications
You must be signed in to change notification settings - Fork 625
Feat: getNextAliveNode() to failover to alternate endpoints on retryable failures. #2880
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
58d96c7
881ecfb
f4a17cd
790cdeb
96934a9
21f71f5
512c0e0
57b01a4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -594,7 +594,10 @@ private ClassicHttpResponse doPostRequest(Map<String, Object> requestConfig, Htt | |
| throw new ClientMisconfigurationException("Proxy authentication required. Please check your proxy settings."); | ||
| } else if (httpResponse.getCode() == HttpStatus.SC_BAD_GATEWAY) { | ||
| httpResponse.close(); | ||
| throw new ClientException("Server returned '502 Bad gateway'. Check network and proxy settings."); | ||
| throw new ConnectException("Server returned '502 Bad gateway'. Check network and proxy settings."); | ||
| } else if (httpResponse.getCode() == HttpStatus.SC_SERVICE_UNAVAILABLE) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please make it a ConnectException. It will be a breaking changes in behavior but we need to fix it because existing behavior is wrong. |
||
| httpResponse.close(); | ||
| throw new ConnectException("Server returned '503 Service unavailable'."); | ||
| } else if (httpResponse.getCode() >= HttpStatus.SC_BAD_REQUEST || httpResponse.containsHeader(ClickHouseHttpProto.HEADER_EXCEPTION_CODE)) { | ||
| try { | ||
| throw readError(req, httpResponse); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| package com.clickhouse.client.api.transport; | ||
|
|
||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
||
| import java.util.ArrayList; | ||
| import java.util.Collections; | ||
| import java.util.List; | ||
|
|
||
| /** | ||
| * So we will look up for the first-alive node and then assign that node to | ||
| * client to talk. | ||
| * | ||
| * <p>Endpoints are tried in the order they were registered (index 0 is the | ||
| * "primary"). When a request fails, the failed endpoint is quarantined for | ||
| * {@link #DEFAULT_QUARANTINE_MS} milliseconds and the next alive endpoint | ||
| * is returned. Once the quarantine expires the node automatically becomes | ||
| * eligible again, so traffic returns to the primary without any explicit | ||
| * reset.</p> | ||
| * | ||
| * <p>If all endpoints are quarantined, the primary (index 0) is returned | ||
| * as a fallback to avoid a complete lockout.</p> | ||
| * | ||
| * <p>This class is thread-safe: concurrent callers may invoke | ||
| * {@link #getEndpoint()} and {@link #getNextAliveNode(Endpoint)} | ||
| * from different threads.</p> | ||
| */ | ||
| public class ClientNodeSelector { | ||
|
|
||
| private static final Logger LOG = LoggerFactory.getLogger(ClientNodeSelector.class); | ||
|
|
||
| static final long DEFAULT_QUARANTINE_MS = 30_000; | ||
|
|
||
| private final List<EndpointState> endpointStates; | ||
|
|
||
| public ClientNodeSelector(List<Endpoint> endpoints) { | ||
| List<EndpointState> states = new ArrayList<>(endpoints.size()); | ||
| for (Endpoint ep : endpoints) { | ||
| states.add(new EndpointState(ep)); | ||
| } | ||
| this.endpointStates = Collections.unmodifiableList(states); | ||
| } | ||
|
|
||
| public Endpoint getEndpoint() { | ||
| for (EndpointState state : endpointStates) { | ||
| if (state.isAlive()) { | ||
| return state.getEndpoint(); | ||
| } | ||
| } | ||
| LOG.warn("All endpoints are non-responsive, falling back to primary endpoint"); | ||
| return endpointStates.get(0).getEndpoint(); | ||
| } | ||
|
|
||
| public Endpoint getNextAliveNode(Endpoint failedEndpoint) { | ||
| for (EndpointState state : endpointStates) { | ||
| if (state.getEndpoint().equals(failedEndpoint)) { | ||
| state.markFailed(DEFAULT_QUARANTINE_MS); | ||
| LOG.warn("Endpoint {} quarantined for {} ms", failedEndpoint.getHost(), DEFAULT_QUARANTINE_MS); | ||
| break; | ||
| } | ||
| } | ||
| return getEndpoint(); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| package com.clickhouse.client.api.transport; | ||
|
|
||
| /** | ||
| *{@link Endpoint} is wrapped to track for failover. | ||
| * When a node fails, it can be quarantined for a fixed duration and after | ||
| * it is expired, it will be considered as alive again. | ||
| */ | ||
| class EndpointState { | ||
|
|
||
| private final Endpoint endpoint; | ||
|
|
||
| private volatile long failedUntil; | ||
|
|
||
| EndpointState(Endpoint endpoint) { | ||
| this.endpoint = endpoint; | ||
| this.failedUntil = 0; | ||
| } | ||
|
|
||
| Endpoint getEndpoint() { | ||
| return endpoint; | ||
| } | ||
|
|
||
| void markFailed(long quarantineMs) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there should be check for negative and minimal value. |
||
| if (quarantineMs <= 0) { | ||
| throw new IllegalArgumentException("Quarantine duration must be positive: " + quarantineMs); | ||
| } | ||
| this.failedUntil = System.currentTimeMillis() + quarantineMs; | ||
| } | ||
|
|
||
| boolean isAlive() { | ||
| return System.currentTimeMillis() >= failedUntil; | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.