AndrewJSchofield commented on code in PR #14111:
URL: https://github.com/apache/kafka/pull/14111#discussion_r1279051318
##########
clients/src/main/java/org/apache/kafka/clients/CommonClientConfigs.java:
##########
@@ -203,18 +224,43 @@ public class CommonClientConfigs {
* @return The new values which have been set as
described in postProcessParsedConfig.
*/
public static Map<String, Object>
postProcessReconnectBackoffConfigs(AbstractConfig config,
- Map<String, Object>
parsedValues) {
+
Map<String, Object> parsedValues) {
HashMap<String, Object> rval = new HashMap<>();
Map<String, Object> originalConfig = config.originals();
if ((!originalConfig.containsKey(RECONNECT_BACKOFF_MAX_MS_CONFIG)) &&
originalConfig.containsKey(RECONNECT_BACKOFF_MS_CONFIG)) {
- log.debug("Disabling exponential reconnect backoff because {} is
set, but {} is not.",
+ log.warn("Disabling exponential reconnect backoff because {} is
set, but {} is not.",
RECONNECT_BACKOFF_MS_CONFIG,
RECONNECT_BACKOFF_MAX_MS_CONFIG);
rval.put(RECONNECT_BACKOFF_MAX_MS_CONFIG,
parsedValues.get(RECONNECT_BACKOFF_MS_CONFIG));
}
return rval;
}
+ /**
+ * Log warning if the exponential backoff is disabled due to initial
backoff value is greater than max backoff value.
+ *
+ * @param config The config object.
+ */
+ public static void warnDisablingExponentialBackoff(AbstractConfig config) {
+ long retryBackoffMs = config.getLong(RETRY_BACKOFF_MS_CONFIG);
+ long retryBackoffMaxMs = config.getLong(RETRY_BACKOFF_MAX_MS_CONFIG);
+ if (retryBackoffMs > retryBackoffMaxMs) {
+ log.warn("Configuration '{}' with value '{}' is greater than
configuration '{}' with value '{}'. " +
+ "A static backoff with value '{}' will be applied.",
+ RETRY_BACKOFF_MS_CONFIG, retryBackoffMs,
+ RETRY_BACKOFF_MAX_MS_CONFIG, retryBackoffMaxMs,
retryBackoffMs);
Review Comment:
Digging into this, I discovered that KIP-601 was not quite implemented
correctly and the backoff value could exceed the maximum either because of
jitter, or because of a lower value for the max config. I'd just followed the
same path for KIP-580, which is not correct.
I have fixed the exponential backoff code for both of these cases so that
the maximum is strictly enforced.
Personally, I think that it's preferable if it is possible for the maximum
to be exceeded because of jitter, but that is not what the KIP says.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
