----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16095/#review29935 -----------------------------------------------------------
Ship it! Just one minor comment below. core/src/main/scala/kafka/controller/ControllerChannelManager.scala <https://reviews.apache.org/r/16095/#comment57416> core/src/main/scala/kafka/controller/ControllerChannelManager.scala <https://reviews.apache.org/r/16095/#comment57417> core/src/test/scala/unit/kafka/admin/AdminTest.scala <https://reviews.apache.org/r/16095/#comment57424> Could we write a separate method to share the code? - Jun Rao On Dec. 6, 2013, 10:49 p.m., Neha Narkhede wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/16095/ > ----------------------------------------------------------- > > (Updated Dec. 6, 2013, 10:49 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1170 > https://issues.apache.org/jira/browse/KAFKA-1170 > > > Repository: kafka > > > Description > ------- > > Fixing unit tests that had incorrect value for replicaFetchWaitMaxMs > > > KAFKA-1170 ISR can be inconsistent during partition reassignment for low > throughput partitions; Changes include 1) Fixing the ISR shrinking logic to > remove replicas that haven't sent a fetch request for > replica.lag.max.wait.ms. 2) Adding a check for replica.fetch.wait.ms <= > replica.lag.max.wait.ms for preventing frequent shrinking and protecting > against misconfiguration 3) Fixing the ISR shrink logic on controller to > always shrink ISR without doing a check in the local cache since the local > cache could be behind when the leader has expanded the ISR 4) Fixed the state > change logging to include relevant information useful for troubleshooting 5) > Fixed the unit tests to check if post reassignment there are extra replicas > in the ISR > > > Diffs > ----- > > core/src/main/scala/kafka/cluster/Partition.scala > 02ccc17c79b6d44c75f9bb6ca7cda8c51ae6f6fb > core/src/main/scala/kafka/controller/ControllerChannelManager.scala > beca460dfe0f4df5ccd7f6358e44cbe742d256e5 > core/src/main/scala/kafka/controller/ReplicaStateMachine.scala > c52225a80ecacead694492fb1525ae60561595a1 > core/src/main/scala/kafka/server/KafkaConfig.scala > 8f9db105898952a51b797a5c314435f6320c92d7 > core/src/main/scala/kafka/server/ReplicaManager.scala > f9c7c29bb96a63d6452269f46a12751daca15b62 > core/src/test/scala/unit/kafka/admin/AdminTest.scala > c30069e837e54fb91bf1d5b75b133282a28dedf8 > core/src/test/scala/unit/kafka/server/ISRExpirationTest.scala > 702643246939f4f734013c8a8be82f45ec34b67a > core/src/test/scala/unit/kafka/server/LogRecoveryTest.scala > 34e39e75e039a14a2426225c28049e72090484df > core/src/test/scala/unit/kafka/server/SimpleFetchTest.scala > bab436dcef1645b5e327a5e7e68abdbe57604745 > > Diff: https://reviews.apache.org/r/16095/diff/ > > > Testing > ------- > > > Thanks, > > Neha Narkhede > >