> On Feb. 3, 2015, 7:14 p.m., Guozhang Wang wrote: > > clients/src/main/java/org/apache/kafka/common/protocol/ApiKeys.java, line 44 > > <https://reviews.apache.org/r/29301/diff/7/?file=821315#file821315line44> > > > > How about just augmenting OFFSET_FETCH request to return offsets > > committed by others within the same group?
We decided to remove ConsumerGroupOffsets tool to a separate ticket / KIP to define a comprehensive list of fields that people would want to see in it. > On Feb. 3, 2015, 7:14 p.m., Guozhang Wang wrote: > > clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java, lines > > 498-542 > > <https://reviews.apache.org/r/29301/diff/7/?file=821316#file821316line498> > > > > It seems we can replace with request with OFFSET_FETCH that can get all > > consumer offsets within the group. And the log-end-offset can be a separate > > and useful request by itself: we just need to combine these two requests to > > get the lag. See above - will be moved to a separate ticket / KIP. > On Feb. 3, 2015, 7:14 p.m., Guozhang Wang wrote: > > core/src/main/scala/kafka/api/ApiUtils.scala, line 45 > > <https://reviews.apache.org/r/29301/diff/7/?file=821349#file821349line45> > > > > "reads a list of values of type T" Removed this code since we declined MaybeOf idea. > On Feb. 3, 2015, 7:14 p.m., Guozhang Wang wrote: > > core/src/main/scala/kafka/api/ApiUtils.scala, line 67 > > <https://reviews.apache.org/r/29301/diff/7/?file=821349#file821349line67> > > > > "reads a single value of type T" See above - removed this code > On Feb. 3, 2015, 7:14 p.m., Guozhang Wang wrote: > > core/src/main/scala/kafka/controller/ControllerChannelManager.scala, lines > > 301-310 > > <https://reviews.apache.org/r/29301/diff/7/?file=821376#file821376line301> > > > > Do not understand the rationale behind this: could you add some > > comments? Particularly, why we want to send an empty metadata map to the > > brokers with forceSendBrokerInfo? > > Andrii Biletskyi wrote: > Thanks, this is done because on startup we don't send UpdateMetadaRequest > (updateMetadataRequestMap is empty) and thus brokers' cache is not filled > with brokers and controller. This leads to ClusterMetadataRequest can't be > served correctly. > I'm not sure this is the best way to do it, open for suggestions. > > Guozhang Wang wrote: > In this case can we just use addUpdateMetadataRequestForBrokers() before > calling sendRequestsToBrokers()? > > Andrii Biletskyi wrote: > If I understood correctly - addUpdateMetadataRequestForBrokers() is > already called, it's just nothing is added to UpdateMetadata. The steps are > the following: > 1. One broker cluster is started (no topics) > 2. KafkaController.onControllerFailover() is called > 3. sendUpdateMetadataRequest() > 4. addUpdateMetadataRequest(): updateMetadataRequest is created foreach > controllerContext.partitionLeadershipInfo.keySet (which is empty) > 5. sendRequestsToBrokers(): we send UpdateMetadata foreach broker from > updateMetadataRequestMap (which is empty) -> broker holding a controller's > role doesn't receive UpdateMetadataRequest > > So essentially the problem is that UpdateMetadaRequest holds data about > controller, brokers _and_ partitionState but we send UpdateMetadaRequest only > if there is partitionState update to be sent. This is was fixed in KAFKA-1867 (liveBroker list not updated on a cluster with no topics) - I removed my workaround after rebase > On Feb. 3, 2015, 7:14 p.m., Guozhang Wang wrote: > > core/src/main/scala/kafka/controller/KafkaController.scala, lines 973-976 > > <https://reviews.apache.org/r/29301/diff/7/?file=821377#file821377line973> > > > > Ditto above, would be better if some comments are added. See above - removed this code. - Andrii ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29301/#review70790 ----------------------------------------------------------- On March 12, 2015, 11:04 a.m., Andrii Biletskyi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29301/ > ----------------------------------------------------------- > > (Updated March 12, 2015, 11:04 a.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1694 > https://issues.apache.org/jira/browse/KAFKA-1694 > > > Repository: kafka > > > Description > ------- > > KAFKA-1694 - introduced new type for Wire protocol, ported > ClusterMetadataResponse to it > > > KAFKA-1694 - Split Admin RQ/RP to separate messages > > > KAFKA-1694 - Admin commands can be handled only by controller; > DeleteTopicCommand NPE fix > > > KAFKA-1776 - Ported ConsumerGroupOffsetChecker > > > KAFKA-1776 - Ported PreferredReplicaElectionTool and ReassignPartitionsTool > to CLI > > > KAFKA-1694 - ReviewBoard 29301 code review fixes > > > KAFKA-1694 - Data for ReassignPartitions and PreferredReplicaLeaderElection > is in json string > > > KAFKA-1694 - Added logging > > > KAFKA-1694 - fixed misprint in schema > > > KAFKA-1694 - DescribeTopicCommand supports all flags that TopicCommand does > > > KAFKA-1694 - Fixed compile error for new Selector constructor > > > KAFKA-1694 - Fixed ConsumerGroupChecker sends DescribeTopicResponse instead > of ConsumerGroupOffsetsResponse > > > KAFKA-1694 - Introduced AbstractAdminRequest/Response to avoid code > duplication for sending admin requests / receiving response. > > > KAFKA-1694 - Code review fix: /core shouldn't depend on /tools > > > KAFKA-1694 - Code review fix: normalized config field in Create- and > AlterTopicRequest > > > KAFKA-1694 - Remove server RQ/RP messages, clients' classes are used instead > > > KAFKA-1694 - Remove ConsumerGroupOffsets RQ/RP - a separate KIP will be > created > > > KAFKA-1694 - Remove MaybeOf type, clean up dead code > > > KAFKA-1694 - Post rebase merge conflicts fixes > > > KAFKA-1694 - Added Protocol errors > > > KAFKA-1694 - Bugfix - incorrect error > > > Diffs > ----- > > bin/kafka.sh PRE-CREATION > bin/windows/kafka.bat PRE-CREATION > build.gradle 0f0fe60a74542efa91a0e727146e896edcaa38af > checkstyle/import-control.xml cca4b38ec766028a604f88a1c63228e40df24573 > clients/src/main/java/org/apache/kafka/common/ConfigEntry.java PRE-CREATION > clients/src/main/java/org/apache/kafka/common/protocol/ApiKeys.java > 07aba71303bc1303dbe05e4b121f73f7ad27fdb5 > clients/src/main/java/org/apache/kafka/common/protocol/Errors.java > ce18a6ce7ed31420cdbec6926a9cd04fa4c806b1 > clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java > 101f382170ad6740b3f8ff2d27b93a64874a857f > > clients/src/main/java/org/apache/kafka/common/requests/ClusterMetadataRequest.java > PRE-CREATION > > clients/src/main/java/org/apache/kafka/common/requests/ClusterMetadataResponse.java > PRE-CREATION > > clients/src/main/java/org/apache/kafka/common/requests/admin/AbstractAdminRequest.java > PRE-CREATION > > clients/src/main/java/org/apache/kafka/common/requests/admin/AbstractAdminResponse.java > PRE-CREATION > > clients/src/main/java/org/apache/kafka/common/requests/admin/AlterTopicRequest.java > PRE-CREATION > > clients/src/main/java/org/apache/kafka/common/requests/admin/AlterTopicResponse.java > PRE-CREATION > > clients/src/main/java/org/apache/kafka/common/requests/admin/CreateTopicRequest.java > PRE-CREATION > > clients/src/main/java/org/apache/kafka/common/requests/admin/CreateTopicResponse.java > PRE-CREATION > > clients/src/main/java/org/apache/kafka/common/requests/admin/DeleteTopicRequest.java > PRE-CREATION > > clients/src/main/java/org/apache/kafka/common/requests/admin/DeleteTopicResponse.java > PRE-CREATION > > clients/src/main/java/org/apache/kafka/common/requests/admin/DescribeTopicOutput.java > PRE-CREATION > > clients/src/main/java/org/apache/kafka/common/requests/admin/DescribeTopicRequest.java > PRE-CREATION > > clients/src/main/java/org/apache/kafka/common/requests/admin/DescribeTopicResponse.java > PRE-CREATION > > clients/src/main/java/org/apache/kafka/common/requests/admin/ListTopicsRequest.java > PRE-CREATION > > clients/src/main/java/org/apache/kafka/common/requests/admin/ListTopicsResponse.java > PRE-CREATION > > clients/src/main/java/org/apache/kafka/common/requests/admin/PreferredReplicaLeaderElectionRequest.java > PRE-CREATION > > clients/src/main/java/org/apache/kafka/common/requests/admin/PreferredReplicaLeaderElectionResponse.java > PRE-CREATION > > clients/src/main/java/org/apache/kafka/common/requests/admin/ReassignPartitionsRequest.java > PRE-CREATION > > clients/src/main/java/org/apache/kafka/common/requests/admin/ReassignPartitionsResponse.java > PRE-CREATION > > clients/src/main/java/org/apache/kafka/common/requests/admin/TopicConfigDetails.java > PRE-CREATION > > clients/src/main/java/org/apache/kafka/common/requests/admin/TopicPartitionDetails.java > PRE-CREATION > > clients/src/main/java/org/apache/kafka/common/requests/admin/VerifyReassignPartitionsRequest.java > PRE-CREATION > > clients/src/main/java/org/apache/kafka/common/requests/admin/VerifyReassignPartitionsResponse.java > PRE-CREATION > > clients/src/test/java/org/apache/kafka/common/requests/RequestResponseTest.java > 13237fd72da5448a3d596b882fef141f336f827d > config/tools-log4j.properties 52f07c96019b4083fc78f62cfb0a81080327e847 > core/src/main/scala/kafka/api/ApiUtils.scala > 1f80de1638978901500df808ca5133308c9d1fca > core/src/main/scala/kafka/api/ClusterMetadaRequestAndHeader.scala > PRE-CREATION > core/src/main/scala/kafka/api/ClusterMetadataResponseAndHeader.scala > PRE-CREATION > core/src/main/scala/kafka/api/RequestKeys.scala > c24c0345feedc7b9e2e9f40af11bfa1b8d328c43 > core/src/main/scala/kafka/api/admin/AlterTopicRequestAndHeader.scala > PRE-CREATION > core/src/main/scala/kafka/api/admin/AlterTopicResponseAndHeader.scala > PRE-CREATION > core/src/main/scala/kafka/api/admin/CreateTopicRequestAndHeader.scala > PRE-CREATION > core/src/main/scala/kafka/api/admin/CreateTopicResponseAndHeader.scala > PRE-CREATION > core/src/main/scala/kafka/api/admin/DeleteTopicRequestAndHeader.scala > PRE-CREATION > core/src/main/scala/kafka/api/admin/DeleteTopicResponseAndHeader.scala > PRE-CREATION > core/src/main/scala/kafka/api/admin/DescribeTopicRequestAndHeader.scala > PRE-CREATION > core/src/main/scala/kafka/api/admin/DescribeTopicResponseAndHeader.scala > PRE-CREATION > core/src/main/scala/kafka/api/admin/ListTopicsRequestAndHeader.scala > PRE-CREATION > core/src/main/scala/kafka/api/admin/ListTopicsResponseAndHeader.scala > PRE-CREATION > > core/src/main/scala/kafka/api/admin/PreferredReplicaLeaderElectionRequestAndHeader.scala > PRE-CREATION > > core/src/main/scala/kafka/api/admin/PreferredReplicaLeaderElectionResponseAndHeader.scala > PRE-CREATION > > core/src/main/scala/kafka/api/admin/ReassignPartitionsRequestAndHeader.scala > PRE-CREATION > > core/src/main/scala/kafka/api/admin/ReassignPartitionsResponseAndHeader.scala > PRE-CREATION > > core/src/main/scala/kafka/api/admin/VerifyReassignPartitionsRequestAndHeader.scala > PRE-CREATION > > core/src/main/scala/kafka/api/admin/VerifyReassignPartitionsResponseAndHeader.scala > PRE-CREATION > core/src/main/scala/kafka/common/AdminRequestFailedException.scala > PRE-CREATION > core/src/main/scala/kafka/common/ErrorMapping.scala > eb1eb4a703098253d0aae79577084569177768d1 > > core/src/main/scala/kafka/common/NotControllerReceivedAdminRequestException.scala > PRE-CREATION > core/src/main/scala/kafka/controller/ControllerChannelManager.scala > c582191636f6188c25d62a67ff0315b56f163133 > core/src/main/scala/kafka/server/KafkaApis.scala > 35af98f0bc1b6a50bd1d97a30147593f8c6a422d > core/src/main/scala/kafka/server/MetadataCache.scala > 6aef6e4508ecadbbcc1e12bed2054547b7aa333e > core/src/main/scala/kafka/server/TopicCommandHelper.scala PRE-CREATION > core/src/main/scala/kafka/tools/PreferredReplicaLeaderElectionHelper.scala > PRE-CREATION > core/src/main/scala/kafka/tools/ReassignPartitionsHelper.scala PRE-CREATION > core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala > fba852afa1b2f46b61e2fd12c38c821ba04e9cc6 > settings.gradle 83f764e6a4a15a5fdba232dce74a369870f26b45 > tools/src/main/java/org/apache/kafka/cli/BaseCommandOpts.java PRE-CREATION > tools/src/main/java/org/apache/kafka/cli/Boot.java PRE-CREATION > tools/src/main/java/org/apache/kafka/cli/RequestDispatcher.java > PRE-CREATION > tools/src/main/java/org/apache/kafka/cli/Shell.java PRE-CREATION > tools/src/main/java/org/apache/kafka/cli/command/AlterTopicCommand.java > PRE-CREATION > tools/src/main/java/org/apache/kafka/cli/command/ClearScreenCommand.java > PRE-CREATION > tools/src/main/java/org/apache/kafka/cli/command/Command.java PRE-CREATION > tools/src/main/java/org/apache/kafka/cli/command/CreateTopicCommand.java > PRE-CREATION > tools/src/main/java/org/apache/kafka/cli/command/DeleteTopicCommand.java > PRE-CREATION > tools/src/main/java/org/apache/kafka/cli/command/DescribeTopicCommand.java > PRE-CREATION > tools/src/main/java/org/apache/kafka/cli/command/ExitCommand.java > PRE-CREATION > tools/src/main/java/org/apache/kafka/cli/command/ListTopicsCommand.java > PRE-CREATION > > tools/src/main/java/org/apache/kafka/cli/command/PreferredReplicaLeaderElectionCommand.java > PRE-CREATION > tools/src/main/java/org/apache/kafka/cli/command/PrintHelpCommand.java > PRE-CREATION > > tools/src/main/java/org/apache/kafka/cli/command/ReassignPartitionsCommand.java > PRE-CREATION > tools/src/main/java/org/apache/kafka/cli/command/TopicSwitchCommand.java > PRE-CREATION > tools/src/main/java/org/apache/kafka/cli/util/StringUtils.java PRE-CREATION > > Diff: https://reviews.apache.org/r/29301/diff/ > > > Testing > ------- > > > Thanks, > > Andrii Biletskyi > >