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

Reply via email to