jsancio commented on a change in pull request #11733:
URL: https://github.com/apache/kafka/pull/11733#discussion_r805920416
##########
File path: core/src/main/scala/kafka/server/AlterIsrManager.scala
##########
@@ -250,23 +260,35 @@ class DefaultAlterIsrManager(
val partitionResponses: mutable.Map[TopicPartition, Either[Errors,
LeaderAndIsr]] =
new mutable.HashMap[TopicPartition, Either[Errors, LeaderAndIsr]]()
data.topics.forEach { topic =>
- topic.partitions().forEach(partition => {
+ topic.partitions().forEach { partition =>
val tp = new TopicPartition(topic.name, partition.partitionIndex)
- val error = Errors.forCode(partition.errorCode())
+ val apiError = Errors.forCode(partition.errorCode())
debug(s"Controller successfully handled AlterIsr request for $tp:
$partition")
- if (error == Errors.NONE) {
- val newLeaderAndIsr = new LeaderAndIsr(partition.leaderId,
partition.leaderEpoch,
- partition.isr.asScala.toList.map(_.toInt),
partition.currentIsrVersion)
- partitionResponses(tp) = Right(newLeaderAndIsr)
+ if (apiError == Errors.NONE) {
+ try {
+ partitionResponses(tp) = Right(
+ LeaderAndIsr(
+ partition.leaderId,
+ partition.leaderEpoch,
+ partition.isr.asScala.toList.map(_.toInt),
+ LeaderRecoveryState.of(partition.leaderRecoveryState),
+ partition.partitionEpoch
+ )
+ )
+ } catch {
+ case e: IllegalArgumentException =>
Review comment:
Yeah. I went back and forth of this. There are some code paths were we
want to throw an exception as in most cases this means a bug in Kafka. I think
this was the only case where we want to handle this exception. I'll take a look
again.
--
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
