C0urante commented on a change in pull request #10014:
URL: https://github.com/apache/kafka/pull/10014#discussion_r568653512
##########
File path:
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/distributed/DistributedHerder.java
##########
@@ -381,7 +381,7 @@ public void tick() {
log.debug("Scheduled rebalance at: {} (now: {}
nextRequestTimeoutMs: {}) ",
scheduledRebalance, now, nextRequestTimeoutMs);
}
- if (internalRequestValidationEnabled() && keyExpiration <
Long.MAX_VALUE) {
+ if (isLeader() && internalRequestValidationEnabled() && keyExpiration
< Long.MAX_VALUE) {
Review comment:
There's a rationale for leaving the check in here but I'm not sold
either way. Let me know what you think:
The `Scheduled next key rotation at...` log message should ideally only be
emitted if key rotation is actually enabled. Key rotation can be [disabled by
setting the ttl to
0](https://github.com/apache/kafka/blob/821d867c1bd01b88aa695827cb57c9aa764857a4/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/distributed/DistributedHerder.java#L1579).
If someone has key rotation disabled, emitting this log message doesn't make a
lot of sense (even if the scheduled rotation time is millions of years in the
future).
This also mirrors the structure of the scheduled rebalance delay check
above, although that check is more necessary because of the tweaking of the
`rebalanceResolved` field.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
