hachikuji commented on code in PR #12181:
URL: https://github.com/apache/kafka/pull/12181#discussion_r894140777
##########
core/src/main/scala/kafka/cluster/Partition.scala:
##########
@@ -1571,14 +1620,26 @@ class Partition(val topicPartition: TopicPartition,
error match {
case Errors.OPERATION_NOT_ATTEMPTED =>
// Since the operation was not attempted, it is safe to reset back to
the committed state.
- partitionState = CommittedPartitionState(proposedIsrState.isr,
LeaderRecoveryState.RECOVERED)
+ partitionState = proposedIsrState.lastCommittedState
Review Comment:
How about we collapse these two cases into one `case`. Then we can add a
comment like this:
```scala
// Care must be taken when resetting to the last committed state since we
may not
// know in general whether the request was applied or not taking into
account retries
// and controller changes which might have occurred before we received the
response.
// However, when the controller returns INELIGIBLE_REPLICA (or
OPERATION_NOT_COMMITTED),
// the controller is explicitly telling us 1) that the current partition
epoch is correct,
// and 2) that the request was not applied. Even if the controller that sent
the response
// is stale, we are guaranteed from the monotonicity of the controller epoch
that the
// request could not have been applied by any past or future controller.
```
--
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]
