Skip to content

Conversation

@caoccao
Copy link
Member

@caoccao caoccao commented Nov 21, 2025

@cla-bot cla-bot bot added the cla-signed label Nov 21, 2025
@caoccao caoccao requested a review from a team November 24, 2025 15:17
Copy link
Contributor

@marregui marregui left a comment

Choose a reason for hiding this comment

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

LGTM

}

final double backoffDelay =
backoffDelays[index] * (1 + new Random().nextDouble(ConnectionOptions.DEFAULT_RETRY_JITTER));
Copy link
Contributor

Choose a reason for hiding this comment

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

a new random object per retry seems excessive, why not use ThreadLocalRandom.current()

final long retryIntervalMs = config.getConnectionOptions().retryIntervalMs();
// Increment retry attempt counter and calculate backoff delay
final int attemptCount = consecutiveRetryAttempts.updateAndGet(count -> count + 1);
long backoffDelayMs ;
Copy link
Contributor

Choose a reason for hiding this comment

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

the extra space before the ; killed my day

@ModuleConfigField(title = "Connection Retry Intervals (milliseconds)",
description = "Comma-separated list of backoff delays in milliseconds for connection retry attempts. The adapter will use these delays sequentially for each retry attempt, repeating the last value if attempts exceed the list length.",
defaultValue = DEFAULT_RETRY_INTERVALS)
String retryIntervalMs,
Copy link
Contributor

Choose a reason for hiding this comment

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

this change from Long to String..... radical, I hope it does not break existing config?

private final @NotNull AtomicReference<ScheduledFuture<?>> healthCheckFuture = new AtomicReference<>();

// Retry attempt tracking for exponential backoff
private final @NotNull AtomicReference<Integer> consecutiveRetryAttempts = new AtomicReference<>(0);
Copy link
Contributor

Choose a reason for hiding this comment

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

AtomicInteger is more compact than an AtomicReference to an Integer (and more efficient)

@caoccao caoccao added the do not merge Do not merge until this label is removed. label Nov 28, 2025
@caoccao caoccao force-pushed the bugfix/38364-auto-reconnect-backoff-strategy branch from 791b828 to b33caf4 Compare December 1, 2025 08:22
Copy link
Contributor

@marregui marregui left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed do not merge Do not merge until this label is removed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants