AndrewJSchofield commented on PR #22270: URL: https://github.com/apache/kafka/pull/22270#issuecomment-4558041599
> @AndrewJSchofield while working on the implementation, I realised there's scope for one slight improvement but it's definitely a new KIP. > > A lot of times when debugging dead consumers I discover that clients have misconfigured their `max.poll.records` and `max.poll.interval.ms` (or the defaults don't match their use case). The `CommitFailedException` and HeartbeatRequestManager print a helpful log statement for this use case but it's one more mode of grepping the logs. > > We emit metrics around rebalance-time, rebalance-last-seconds-ago, rebalance-total, but I think a useful metric and an enum for the rebalance listeners is a `RebalanceCause` with the other modes like COORDINATOR_INITIATED, JOIN_GROUP, LEAVE_GROUP etc. This pattern mirrors Guava/Caffeine cache semantics for eviction. > > The reason I'm asking now is that we could change the second parameter from a `RebalanceConsumer` to a `RebalanceContext` with a `.consumer()`, `.cause()` and (in the future maybe other fields like memberId if consumers care about it). This helps avoid changing the signature _once more_ if this PR gets merged and reaches 4.4. So I felt now would be a good time to bring this up rather than later. > > WDYT? The idea of having a class which can subsequently be enhanced by a future KIP without an interface change is a good one. The cause part of this is DEFINITELY a new KIP 😆 but `RebalanceContext` could indeed be part of the new interface. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
