lianetm commented on code in PR #15579:
URL: https://github.com/apache/kafka/pull/15579#discussion_r1536155346
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/MembershipManagerImpl.java:
##########
@@ -1010,49 +1015,55 @@ private void revokeAndAssign(LocalAssignment
resolvedAssignment,
// and assignment, executed sequentially).
CompletableFuture<Void> reconciliationResult =
revocationResult.thenCompose(__ -> {
- boolean memberHasRejoined = memberEpochOnReconciliationStart
!= memberEpoch;
- if (state == MemberState.RECONCILING && !memberHasRejoined) {
+ if (!maybeAbortReconciliation()) {
// Apply assignment
return assignPartitions(assignedTopicIdPartitions,
addedPartitions);
} else {
- log.debug("Revocation callback completed but the member
already " +
- "transitioned out of the reconciling state for epoch
{} into " +
- "{} state with epoch {}. Interrupting reconciliation
as it's " +
- "not relevant anymore,",
memberEpochOnReconciliationStart, state, memberEpoch);
- String reason = interruptedReconciliationErrorMessage();
CompletableFuture<Void> res = new CompletableFuture<>();
res.completeExceptionally(new KafkaException("Interrupting
reconciliation" +
Review Comment:
uhm good point, I did consider it but wasn't fully convinced at that time
(mostly taking into account that this abort path is not only in the case of
rejoin while reconciling, but also in the case of fatal failures while
reconciling, for instance). But I do like you point of view about the 2 paths,
seeing from the POV that even in the failure scenario, the logging/error
handling should be done on that fatal transition path, and the reconciliation
itself could only be responsible for aborting quietly. Changed, take a look and
let me know your thoughts (note that the tradeoff is that it requires another
check before sending the ack, to make sure that the reconciliation hasn't been
already aborted)
--
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]
