ijuma commented on code in PR #12741:
URL: https://github.com/apache/kafka/pull/12741#discussion_r994648305


##########
core/src/main/scala/kafka/server/ReplicaManager.scala:
##########
@@ -2151,28 +2150,24 @@ class ReplicaManager(val config: KafkaConfig,
         try {
           followerTopicSet.add(tp.topic)
 
-          if (shuttingDown) {
-            stateChangeLogger.trace(s"Unable to start fetching $tp with topic 
" +
-              s"ID ${info.topicId} because the replica manager is shutting 
down.")
-          } else {
-            // We always update the follower state.
-            // - This ensure that a replica with no leader can step down;
-            // - This also ensures that the local replica is created even if 
the leader
-            //   is unavailable. This is required to ensure that we include 
the partition's
-            //   high watermark in the checkpoint file (see KAFKA-1647).
-            val state = info.partition.toLeaderAndIsrPartitionState(tp, isNew)
-            val isNewLeaderEpoch = partition.makeFollower(state, 
offsetCheckpoints, Some(info.topicId))
-
-            if (isInControlledShutdown && (info.partition.leader == NO_LEADER 
||

Review Comment:
   Similarly, why do we need this special `isInControlledShutdown` field versus 
relying on the broker state as I mentioned above? This kind of approach is 
extremely brittle in my opinion.



-- 
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

Reply via email to