You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Joel Koshy <jj...@gmail.com> on 2014/04/04 20:22:55 UTC
Review Request 20038: Patch for KAFKA-1355
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20038/
-----------------------------------------------------------
Review request for kafka.
Bugs: KAFKA-1355
https://issues.apache.org/jira/browse/KAFKA-1355
Repository: kafka
Description
-------
Avoid sending all topic metadata on state changes.
Diffs
-----
core/src/main/scala/kafka/api/LeaderAndIsrRequest.scala 03117377e1cb2ad63e4c7740d97ca9a4f20abeeb
core/src/main/scala/kafka/controller/ControllerChannelManager.scala f17d97603798e803d035f313f0165f11f6d0f2c0
core/src/main/scala/kafka/controller/KafkaController.scala fcabd0da201ccd478f5884bcca9b75a8c8ea8373
core/src/main/scala/kafka/controller/TopicDeletionManager.scala 09f54acc733d49a9be9a9c6633271c190dea1bf0
core/src/main/scala/kafka/server/KafkaApis.scala c068ef69207c351eec413a595f1747c59f8b3983
Diff: https://reviews.apache.org/r/20038/diff/
Testing
-------
Thanks,
Joel Koshy
Re: Review Request 20038: Patch for KAFKA-1355
Posted by Neha Narkhede <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20038/#review39614
-----------------------------------------------------------
Ship it!
Ship It!
- Neha Narkhede
On April 4, 2014, 8:51 p.m., Joel Koshy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20038/
> -----------------------------------------------------------
>
> (Updated April 4, 2014, 8:51 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1355
> https://issues.apache.org/jira/browse/KAFKA-1355
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Avoid sending all topic metadata on state changes.
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/api/LeaderAndIsrRequest.scala 03117377e1cb2ad63e4c7740d97ca9a4f20abeeb
> core/src/main/scala/kafka/controller/ControllerChannelManager.scala f17d97603798e803d035f313f0165f11f6d0f2c0
> core/src/main/scala/kafka/controller/KafkaController.scala fcabd0da201ccd478f5884bcca9b75a8c8ea8373
> core/src/main/scala/kafka/controller/TopicDeletionManager.scala 09f54acc733d49a9be9a9c6633271c190dea1bf0
> core/src/main/scala/kafka/server/KafkaApis.scala c068ef69207c351eec413a595f1747c59f8b3983
>
> Diff: https://reviews.apache.org/r/20038/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Joel Koshy
>
>
Re: Review Request 20038: Patch for KAFKA-1355
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20038/#review39616
-----------------------------------------------------------
Ship it!
Ship It!
- Timothy Chen
On April 4, 2014, 8:51 p.m., Joel Koshy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20038/
> -----------------------------------------------------------
>
> (Updated April 4, 2014, 8:51 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1355
> https://issues.apache.org/jira/browse/KAFKA-1355
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Avoid sending all topic metadata on state changes.
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/api/LeaderAndIsrRequest.scala 03117377e1cb2ad63e4c7740d97ca9a4f20abeeb
> core/src/main/scala/kafka/controller/ControllerChannelManager.scala f17d97603798e803d035f313f0165f11f6d0f2c0
> core/src/main/scala/kafka/controller/KafkaController.scala fcabd0da201ccd478f5884bcca9b75a8c8ea8373
> core/src/main/scala/kafka/controller/TopicDeletionManager.scala 09f54acc733d49a9be9a9c6633271c190dea1bf0
> core/src/main/scala/kafka/server/KafkaApis.scala c068ef69207c351eec413a595f1747c59f8b3983
>
> Diff: https://reviews.apache.org/r/20038/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Joel Koshy
>
>
Re: Review Request 20038: Patch for KAFKA-1355
Posted by Jun Rao <ju...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20038/#review39798
-----------------------------------------------------------
core/src/main/scala/kafka/controller/ControllerChannelManager.scala
<https://reviews.apache.org/r/20038/#comment72399>
This comment seems outdated. Could you make it up to date?
- Jun Rao
On April 4, 2014, 8:51 p.m., Joel Koshy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20038/
> -----------------------------------------------------------
>
> (Updated April 4, 2014, 8:51 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1355
> https://issues.apache.org/jira/browse/KAFKA-1355
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Avoid sending all topic metadata on state changes.
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/api/LeaderAndIsrRequest.scala 03117377e1cb2ad63e4c7740d97ca9a4f20abeeb
> core/src/main/scala/kafka/controller/ControllerChannelManager.scala f17d97603798e803d035f313f0165f11f6d0f2c0
> core/src/main/scala/kafka/controller/KafkaController.scala fcabd0da201ccd478f5884bcca9b75a8c8ea8373
> core/src/main/scala/kafka/controller/TopicDeletionManager.scala 09f54acc733d49a9be9a9c6633271c190dea1bf0
> core/src/main/scala/kafka/server/KafkaApis.scala c068ef69207c351eec413a595f1747c59f8b3983
>
> Diff: https://reviews.apache.org/r/20038/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Joel Koshy
>
>
Re: Review Request 20038: Patch for KAFKA-1355
Posted by Joel Koshy <jj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20038/
-----------------------------------------------------------
(Updated April 4, 2014, 8:51 p.m.)
Review request for kafka.
Bugs: KAFKA-1355
https://issues.apache.org/jira/browse/KAFKA-1355
Repository: kafka
Description
-------
Avoid sending all topic metadata on state changes.
Diffs (updated)
-----
core/src/main/scala/kafka/api/LeaderAndIsrRequest.scala 03117377e1cb2ad63e4c7740d97ca9a4f20abeeb
core/src/main/scala/kafka/controller/ControllerChannelManager.scala f17d97603798e803d035f313f0165f11f6d0f2c0
core/src/main/scala/kafka/controller/KafkaController.scala fcabd0da201ccd478f5884bcca9b75a8c8ea8373
core/src/main/scala/kafka/controller/TopicDeletionManager.scala 09f54acc733d49a9be9a9c6633271c190dea1bf0
core/src/main/scala/kafka/server/KafkaApis.scala c068ef69207c351eec413a595f1747c59f8b3983
Diff: https://reviews.apache.org/r/20038/diff/
Testing
-------
Thanks,
Joel Koshy
Re: Review Request 20038: Patch for KAFKA-1355
Posted by Joel Koshy <jj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20038/
-----------------------------------------------------------
(Updated April 4, 2014, 8:48 p.m.)
Review request for kafka.
Bugs: KAFKA-1355
https://issues.apache.org/jira/browse/KAFKA-1355
Repository: kafka
Description
-------
Avoid sending all topic metadata on state changes.
Diffs (updated)
-----
core/src/main/scala/kafka/api/LeaderAndIsrRequest.scala 03117377e1cb2ad63e4c7740d97ca9a4f20abeeb
core/src/main/scala/kafka/controller/ControllerChannelManager.scala f17d97603798e803d035f313f0165f11f6d0f2c0
core/src/main/scala/kafka/controller/KafkaController.scala fcabd0da201ccd478f5884bcca9b75a8c8ea8373
core/src/main/scala/kafka/controller/TopicDeletionManager.scala 09f54acc733d49a9be9a9c6633271c190dea1bf0
core/src/main/scala/kafka/server/KafkaApis.scala c068ef69207c351eec413a595f1747c59f8b3983
Diff: https://reviews.apache.org/r/20038/diff/
Testing
-------
Thanks,
Joel Koshy
Re: Review Request 20038: Patch for KAFKA-1355
Posted by Joel Koshy <jj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20038/#review39571
-----------------------------------------------------------
core/src/main/scala/kafka/controller/ControllerChannelManager.scala
<https://reviews.apache.org/r/20038/#comment71986>
Quick comment on this:
A better way to do this would be to modify the PartitionStateInfo in
UpdateMetadataRequest to include a "delete" flag and increment the protocol
version for UpdateMetadataRequest. The problem with this is that since these
are intra-cluster requests, roll-back (if required) will be impossible.
This is a general issue with evolving request schemas for intra-cluster
requests/responses. If such changes are needed, then the steps would be:
- Check-in a fix with just the schema changes (#1)
- Check-in feature that uses those changes (#2)
This is assuming that #1 is an innocuous enough change that #1 is almost
equivalent to the version that precedes it. If any performance or
correctness issue is uncovered in #2, then we can roll-back to #1.
I cannot really do the above for this specific issue because the slow-down
due to logging happened a while ago. We can fix forward though - once this
is checked in we can modify the request schema and then implement the better
fix.
- Joel Koshy
On April 4, 2014, 6:22 p.m., Joel Koshy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20038/
> -----------------------------------------------------------
>
> (Updated April 4, 2014, 6:22 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1355
> https://issues.apache.org/jira/browse/KAFKA-1355
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Avoid sending all topic metadata on state changes.
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/api/LeaderAndIsrRequest.scala 03117377e1cb2ad63e4c7740d97ca9a4f20abeeb
> core/src/main/scala/kafka/controller/ControllerChannelManager.scala f17d97603798e803d035f313f0165f11f6d0f2c0
> core/src/main/scala/kafka/controller/KafkaController.scala fcabd0da201ccd478f5884bcca9b75a8c8ea8373
> core/src/main/scala/kafka/controller/TopicDeletionManager.scala 09f54acc733d49a9be9a9c6633271c190dea1bf0
> core/src/main/scala/kafka/server/KafkaApis.scala c068ef69207c351eec413a595f1747c59f8b3983
>
> Diff: https://reviews.apache.org/r/20038/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Joel Koshy
>
>
Re: Review Request 20038: Patch for KAFKA-1355
Posted by Neha Narkhede <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20038/#review39587
-----------------------------------------------------------
core/src/main/scala/kafka/controller/ControllerChannelManager.scala
<https://reviews.apache.org/r/20038/#comment72017>
Can this be renamed to something like updateMetadataRequestMap?
core/src/main/scala/kafka/controller/ControllerChannelManager.scala
<https://reviews.apache.org/r/20038/#comment72016>
There can be overlap in these 2 lists of partitions when large number of topics are being deleted. So instead of potentially updating the overlapping partitions twice, can we separate them first and then invoke updateMapFor with the right value for beingDeleted?
- Neha Narkhede
On April 4, 2014, 6:22 p.m., Joel Koshy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20038/
> -----------------------------------------------------------
>
> (Updated April 4, 2014, 6:22 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1355
> https://issues.apache.org/jira/browse/KAFKA-1355
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Avoid sending all topic metadata on state changes.
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/api/LeaderAndIsrRequest.scala 03117377e1cb2ad63e4c7740d97ca9a4f20abeeb
> core/src/main/scala/kafka/controller/ControllerChannelManager.scala f17d97603798e803d035f313f0165f11f6d0f2c0
> core/src/main/scala/kafka/controller/KafkaController.scala fcabd0da201ccd478f5884bcca9b75a8c8ea8373
> core/src/main/scala/kafka/controller/TopicDeletionManager.scala 09f54acc733d49a9be9a9c6633271c190dea1bf0
> core/src/main/scala/kafka/server/KafkaApis.scala c068ef69207c351eec413a595f1747c59f8b3983
>
> Diff: https://reviews.apache.org/r/20038/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Joel Koshy
>
>