Re: Review Request 29301: Patch for KAFKA-1694
--- 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 (updated) --- 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 (updated) - 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
Re: Review Request 29301: Patch for KAFKA-1694
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
Re: Review Request 29301: Patch for KAFKA-1694
On Feb. 3, 2015, 7:14 p.m., Guozhang Wang wrote:
core/src/main/scala/kafka/server/TopicCommandHelper.scala, lines 1-17
https://reviews.apache.org/r/29301/diff/7/?file=821380#file821380line1
One general comment:
For some topic commands, why use AdminUtils to write ZK path again
instead of handle it via the controller directly? Or this is still WIP?
Andrii Biletskyi wrote:
Not sure I understand you. You mean technially calling ZK client from
Controller class, not through TopicCommandHelper? If so - it's just to leave
KafkaApi clean and small.
Guozhang Wang wrote:
For example, upon receiving a create-topic request, the helper class will
call AdminUtils.createOrUpdateTopicPartitionAssignmentPathInZK() which will
just write this request to ZK admin path for it to be captured by controller;
however since only the broker with the active controller will receive such
requests why don't we just hand off the request from KafkaApi to the
controller to handle it.
One question, though, is that we need to make sure concurrency is correct
for controller handling multiple such tasks, and we have some thoughts about
how to deal with such cases (see Jiangjie and my commnets in KAFKA-1305).
Andrii Biletskyi wrote:
Thanks for explanation.
So instead of current workflow:
CreateTopicRequest - Helper class - AdminUtils - zk path is created -
Controller's changeTopicListener picks up the change - topic is created
You propose:
CreateTopicRequest - Controller directly executes logic from
ChangeTopicListener
?
Very interesting idea! Can we make a separate ticket for that? I tried to
port TopicCommand as is in order to have at least for now working
end-to-end infrastructure to handle Admin commands. I believe this is more
like refactoring TopicCommand (probably delete- and alterTopic should be
changed too). I'm a bit concerned adding this refactoring will require
additional efforts to test (especially taking into account your note about
KAFKA-1305) and time to agree on approach we will use to address this issue.
Agree.
On Feb. 3, 2015, 7:14 p.m., Guozhang Wang wrote:
clients/src/main/java/org/apache/kafka/common/requests/admin/AbstractAdminRequest.java,
lines 1-28
https://reviews.apache.org/r/29301/diff/7/?file=821321#file821321line1
Wondering if an abstract admin request is necessary, as it does not
have many common interface functions.
Andrii Biletskyi wrote:
This is needed to avoid code dupliaction in admin clients. See
RequestDispatcher for example.
You will need to call admin request and get response of that type. Having
AbstractAdminRequest (specifically createResponseCounterpart) lets you have:
```
public T extends AbstractAdminResponse T
sendAdminRequest(AbstractAdminRequestT abstractRequest) throws Exception {
```
Instead of sendCreateTopicRequest, sendAlter... etc. If there is a better
and cleaner way to achive this - please let me know.
Guozhang Wang wrote:
I see. How about changing sendAdminRequest(AbstractAdminRequestT) to
sendRequest(ClientRequest) and the caller like AlterTopicCommand.execute()
will be:
AlterTopicRequest alterTopicRequest = // create the request
ClientRequest request = new ClientRequest(new RequestSend(...) ...)
dispatcher.sendRequest(request)
This way we are duplicating the second line here in every upper-level
class, while saving the admin interface. I actually do not know which one is
better..
Andrii Biletskyi wrote:
Yes, but you will also need typed response. Let me continue your example:
AlterTopicRequest alterTopicRequest = // create the request
ClientRequest request = new ClientRequest(new RequestSend(...) ...)
__ClientResponse response = dispatcher.sendRequest(request,
ApiKeys.ALTER_TOPIC)__
__AlterTopicResponse alterTopicResponse = new
AlterTopicResponse(response.responseBody())__
alterTopicResponse.// now get what you need from typed response
And you will have this NetworkClient related Stuff (RequestSend,
ClientRequest ...) everywhere in you client code. But it looks pretty strange
you can't have generic method to send request and get immidiately response of
the required type.
So really RequestDispatcher allready has sendRequest() as you suggest,
with sendAdminRequest I tried to address issue with getting response
counterpart. But I agree that solution might mislead people, so if doesn't
worth it - I'm okay to remove intermediate AbstractAdminRequest/Response.
Makes sense, I am now OK with the admin request interface.
- Guozhang
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29301/#review70790
---
On Jan. 14, 2015,
Re: Review Request 29301: Patch for KAFKA-1694
On Feb. 3, 2015, 7:14 p.m., Guozhang Wang wrote:
core/src/main/scala/kafka/server/TopicCommandHelper.scala, lines 1-17
https://reviews.apache.org/r/29301/diff/7/?file=821380#file821380line1
One general comment:
For some topic commands, why use AdminUtils to write ZK path again
instead of handle it via the controller directly? Or this is still WIP?
Andrii Biletskyi wrote:
Not sure I understand you. You mean technially calling ZK client from
Controller class, not through TopicCommandHelper? If so - it's just to leave
KafkaApi clean and small.
Guozhang Wang wrote:
For example, upon receiving a create-topic request, the helper class will
call AdminUtils.createOrUpdateTopicPartitionAssignmentPathInZK() which will
just write this request to ZK admin path for it to be captured by controller;
however since only the broker with the active controller will receive such
requests why don't we just hand off the request from KafkaApi to the
controller to handle it.
One question, though, is that we need to make sure concurrency is correct
for controller handling multiple such tasks, and we have some thoughts about
how to deal with such cases (see Jiangjie and my commnets in KAFKA-1305).
Thanks for explanation.
So instead of current workflow:
CreateTopicRequest - Helper class - AdminUtils - zk path is created -
Controller's changeTopicListener picks up the change - topic is created
You propose:
CreateTopicRequest - Controller directly executes logic from
ChangeTopicListener
?
Very interesting idea! Can we make a separate ticket for that? I tried to port
TopicCommand as is in order to have at least for now working end-to-end
infrastructure to handle Admin commands. I believe this is more like
refactoring TopicCommand (probably delete- and alterTopic should be changed
too). I'm a bit concerned adding this refactoring will require additional
efforts to test (especially taking into account your note about KAFKA-1305) and
time to agree on approach we will use to address this issue.
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()?
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.
On Feb. 3, 2015, 7:14 p.m., Guozhang Wang wrote:
clients/src/main/java/org/apache/kafka/common/requests/admin/AbstractAdminRequest.java,
lines 1-28
https://reviews.apache.org/r/29301/diff/7/?file=821321#file821321line1
Wondering if an abstract admin request is necessary, as it does not
have many common interface functions.
Andrii Biletskyi wrote:
This is needed to avoid code dupliaction in admin clients. See
RequestDispatcher for example.
You will need to call admin request and get response of that type. Having
AbstractAdminRequest (specifically createResponseCounterpart) lets you have:
```
public T extends AbstractAdminResponse T
sendAdminRequest(AbstractAdminRequestT abstractRequest) throws Exception {
```
Instead of sendCreateTopicRequest, sendAlter... etc. If there is a better
and cleaner way to achive this - please let me know.
Guozhang Wang wrote:
I see. How about changing sendAdminRequest(AbstractAdminRequestT) to
sendRequest(ClientRequest) and the caller like AlterTopicCommand.execute()
will be:
AlterTopicRequest alterTopicRequest = // create the request
ClientRequest request = new
Re: Review Request 29301: Patch for KAFKA-1694
On Feb. 3, 2015, 7:14 p.m., Guozhang Wang wrote:
clients/src/main/java/org/apache/kafka/common/protocol/ApiKeys.java, lines
39-42
https://reviews.apache.org/r/29301/diff/7/?file=821315#file821315line39
How about merge them into one request? The format could be:
topic - string
action - string (create, alter, delete, describe)
num_partition (?) - int32
reassignment (?) - string
configs (?) - list[string]
For configs, the client can first get the current config overriden
list, and get the new configs list as (curr + added - deleted) so that we
do not need two configs field in the request.
Andrii Biletskyi wrote:
I see your point but I'm not sure this will comply Wire protocol use
cases. My understanding is that Wire protocol has schema and RQ/RP message is
always defined and static.
So it's probably possible to merge all requests in one but reusing some
of the fields in different types and thus having them different meaning,
makes such request type stand out from the others.
Also I believe merging all create-alter-delete-describe won't go that
smoothly:
1) 'describe' has very different _response_ schema than mutating
'create-alter-delete'
2) I will probably remove MaybeOf type, as guys had concerns, this is
different how empty values are handled now (see Jay's comment in KIP
discussion) - this will make things even more tangled
3) There was a comment about batching commands - I'll probably modify
schema so it will accept whitelist for topics (as in TopicCommand), but this
is right only for 'alter-delete' - you can't create topic by reqexp - another
issue to find the common point
4) There is also 'replicas' field which is used only in 'create'
As you see messages are really different.
Makes sense, if we are not going the MaybeOf type then there is no point
merging them.
On Feb. 3, 2015, 7:14 p.m., Guozhang Wang wrote:
clients/src/main/java/org/apache/kafka/common/requests/admin/AbstractAdminRequest.java,
lines 1-28
https://reviews.apache.org/r/29301/diff/7/?file=821321#file821321line1
Wondering if an abstract admin request is necessary, as it does not
have many common interface functions.
Andrii Biletskyi wrote:
This is needed to avoid code dupliaction in admin clients. See
RequestDispatcher for example.
You will need to call admin request and get response of that type. Having
AbstractAdminRequest (specifically createResponseCounterpart) lets you have:
```
public T extends AbstractAdminResponse T
sendAdminRequest(AbstractAdminRequestT abstractRequest) throws Exception {
```
Instead of sendCreateTopicRequest, sendAlter... etc. If there is a better
and cleaner way to achive this - please let me know.
I see. How about changing sendAdminRequest(AbstractAdminRequestT) to
sendRequest(ClientRequest) and the caller like AlterTopicCommand.execute()
will be:
AlterTopicRequest alterTopicRequest = // create the request
ClientRequest request = new ClientRequest(new RequestSend(...) ...)
dispatcher.sendRequest(request)
This way we are duplicating the second line here in every upper-level class,
while saving the admin interface. I actually do not know which one is better..
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.
In this case can we just use addUpdateMetadataRequestForBrokers() before
calling sendRequestsToBrokers()?
On Feb. 3, 2015, 7:14 p.m., Guozhang Wang wrote:
core/src/main/scala/kafka/server/TopicCommandHelper.scala, lines 1-17
https://reviews.apache.org/r/29301/diff/7/?file=821380#file821380line1
One general comment:
For some topic commands, why use AdminUtils to write ZK path again
instead of handle it via the controller directly? Or this is still WIP?
Andrii Biletskyi wrote:
Not sure I understand you. You mean technially calling ZK client from
Controller class, not through TopicCommandHelper? If so - it's just to leave
KafkaApi clean and small.
For example, upon receiving a create-topic request, the helper class will call
AdminUtils.createOrUpdateTopicPartitionAssignmentPathInZK() which will just
write this request to ZK admin path for it to be captured by
Re: Review Request 29301: Patch for KAFKA-1694
On Feb. 3, 2015, 7:14 p.m., Guozhang Wang wrote:
clients/src/main/java/org/apache/kafka/common/protocol/ApiKeys.java, lines
39-42
https://reviews.apache.org/r/29301/diff/7/?file=821315#file821315line39
How about merge them into one request? The format could be:
topic - string
action - string (create, alter, delete, describe)
num_partition (?) - int32
reassignment (?) - string
configs (?) - list[string]
For configs, the client can first get the current config overriden
list, and get the new configs list as (curr + added - deleted) so that we
do not need two configs field in the request.
I see your point but I'm not sure this will comply Wire protocol use cases. My
understanding is that Wire protocol has schema and RQ/RP message is always
defined and static.
So it's probably possible to merge all requests in one but reusing some of the
fields in different types and thus having them different meaning, makes such
request type stand out from the others.
Also I believe merging all create-alter-delete-describe won't go that smoothly:
1) 'describe' has very different _response_ schema than mutating
'create-alter-delete'
2) I will probably remove MaybeOf type, as guys had concerns, this is different
how empty values are handled now (see Jay's comment in KIP discussion) - this
will make things even more tangled
3) There was a comment about batching commands - I'll probably modify schema so
it will accept whitelist for topics (as in TopicCommand), but this is right
only for 'alter-delete' - you can't create topic by reqexp - another issue to
find the common point
4) There is also 'replicas' field which is used only in 'create'
As you see messages are really different.
On Feb. 3, 2015, 7:14 p.m., Guozhang Wang wrote:
clients/src/main/java/org/apache/kafka/common/requests/admin/AbstractAdminRequest.java,
lines 1-28
https://reviews.apache.org/r/29301/diff/7/?file=821321#file821321line1
Wondering if an abstract admin request is necessary, as it does not
have many common interface functions.
This is needed to avoid code dupliaction in admin clients. See
RequestDispatcher for example.
You will need to call admin request and get response of that type. Having
AbstractAdminRequest (specifically createResponseCounterpart) lets you have:
```
public T extends AbstractAdminResponse T
sendAdminRequest(AbstractAdminRequestT abstractRequest) throws Exception {
```
Instead of sendCreateTopicRequest, sendAlter... etc. If there is a better and
cleaner way to achive this - please let me know.
On Feb. 3, 2015, 7:14 p.m., Guozhang Wang wrote:
core/src/main/scala/kafka/server/TopicCommandHelper.scala, lines 1-17
https://reviews.apache.org/r/29301/diff/7/?file=821380#file821380line1
One general comment:
For some topic commands, why use AdminUtils to write ZK path again
instead of handle it via the controller directly? Or this is still WIP?
Not sure I understand you. You mean technially calling ZK client from
Controller class, not through TopicCommandHelper? If so - it's just to leave
KafkaApi clean and small.
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?
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.
- Andrii
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29301/#review70790
---
On Jan. 14, 2015, 4:07 p.m., Andrii Biletskyi wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29301/
---
(Updated Jan. 14, 2015, 4:07 p.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
Re: Review Request 29301: Patch for KAFKA-1694
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29301/#review70790 --- Looks good overall. Some comments below, which did not cover the client implementation and the some request helper classes. build.gradle https://reviews.apache.org/r/29301/#comment116223 Why core compilation is dependent on tools compilation? clients/src/main/java/org/apache/kafka/common/protocol/ApiKeys.java https://reviews.apache.org/r/29301/#comment116230 How about merge them into one request? The format could be: topic - string action - string (create, alter, delete, describe) num_partition (?) - int32 reassignment (?) - string configs (?) - list[string] For configs, the client can first get the current config overriden list, and get the new configs list as (curr + added - deleted) so that we do not need two configs field in the request. clients/src/main/java/org/apache/kafka/common/protocol/ApiKeys.java https://reviews.apache.org/r/29301/#comment116225 How about just augmenting OFFSET_FETCH request to return offsets committed by others within the same group? clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java https://reviews.apache.org/r/29301/#comment116231 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. clients/src/main/java/org/apache/kafka/common/requests/ClusterMetadataResponse.java https://reviews.apache.org/r/29301/#comment116232 Rename to controllerStruct? clients/src/main/java/org/apache/kafka/common/requests/admin/AbstractAdminRequest.java https://reviews.apache.org/r/29301/#comment116234 Wondering if an abstract admin request is necessary, as it does not have many common interface functions. clients/src/main/java/org/apache/kafka/common/requests/admin/AbstractAdminResponse.java https://reviews.apache.org/r/29301/#comment116235 Ditto. config/tools-log4j.properties https://reviews.apache.org/r/29301/#comment116236 Is this intentional? WARN = INFO core/src/main/scala/kafka/api/ApiUtils.scala https://reviews.apache.org/r/29301/#comment116237 reads a list of values of type T core/src/main/scala/kafka/api/ApiUtils.scala https://reviews.apache.org/r/29301/#comment116238 reads a single value of type T core/src/main/scala/kafka/controller/ControllerChannelManager.scala https://reviews.apache.org/r/29301/#comment116239 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? core/src/main/scala/kafka/controller/KafkaController.scala https://reviews.apache.org/r/29301/#comment116240 Ditto above, would be better if some comments are added. core/src/main/scala/kafka/server/TopicCommandHelper.scala https://reviews.apache.org/r/29301/#comment116246 One general comment: For some topic commands, why use AdminUtils to write ZK path again instead of handle it via the controller directly? Or this is still WIP? - Guozhang Wang On Jan. 14, 2015, 4:07 p.m., Andrii Biletskyi wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29301/ --- (Updated Jan. 14, 2015, 4:07 p.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 - kafka-tools is uploaded on uploadAllArchives 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. Diffs - bin/kafka.sh PRE-CREATION
Re: Review Request 29301: Patch for KAFKA-1694
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29301/ --- (Updated Jan. 14, 2015, 1:42 p.m.) Review request for kafka. Bugs: KAFKA-1694 https://issues.apache.org/jira/browse/KAFKA-1694 Repository: kafka Description (updated) --- 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 - kafka-tools is uploaded on uploadAllArchives 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 Diffs (updated) - bin/kafka.sh PRE-CREATION bin/windows/kafka.bat PRE-CREATION build.gradle c9ac43378c3bf5443f0f47c8ba76067237ecb348 clients/src/main/java/org/apache/kafka/common/protocol/ApiKeys.java 109fc965e09b2ed186a073351bd037ac8af20a4c clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 7517b879866fc5dad5f8d8ad30636da8bbe7784a clients/src/main/java/org/apache/kafka/common/protocol/types/MaybeOf.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/protocol/types/Struct.java 121e880a941fcd3e6392859edba11a94236494cc 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/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/ConsumerGroupOffsetsRequest.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/requests/admin/ConsumerGroupOffsetsResponse.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/ListTopicsOutput.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/VerifyPreferredReplicaLeaderElectionRequest.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/requests/admin/VerifyPreferredReplicaLeaderElectionResponse.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 df37fc6d8f0db0b8192a948426af603be3444da4 config/tools-log4j.properties 52f07c96019b4083fc78f62cfb0a81080327e847 core/src/main/scala/kafka/api/ApiUtils.scala 1f80de1638978901500df808ca5133308c9d1fca core/src/main/scala/kafka/api/ClusterMetadataRequest.scala PRE-CREATION core/src/main/scala/kafka/api/ClusterMetadataResponse.scala PRE-CREATION
Re: Review Request 29301: Patch for KAFKA-1694
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29301/ --- (Updated Jan. 13, 2015, 5:30 p.m.) Review request for kafka. Bugs: KAFKA-1694 https://issues.apache.org/jira/browse/KAFKA-1694 Repository: kafka Description (updated) --- 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 - kafka-tools is uploaded on uploadAllArchives KAFKA-1694 - ReviewBoard 29301 code review fixes KAFKA-1694 - Data for ReassignPartitions and PreferredReplicaLeaderElection is in json string KAFKA-1694 - Added logging Diffs (updated) - bin/kafka.sh PRE-CREATION bin/windows/kafka.bat PRE-CREATION build.gradle ba52288031e2abc70e35e9297a4423dd5025950b clients/src/main/java/org/apache/kafka/common/protocol/ApiKeys.java 109fc965e09b2ed186a073351bd037ac8af20a4c clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 7517b879866fc5dad5f8d8ad30636da8bbe7784a clients/src/main/java/org/apache/kafka/common/protocol/types/MaybeOf.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/protocol/types/Struct.java 121e880a941fcd3e6392859edba11a94236494cc 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/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/ConsumerGroupOffsetsRequest.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/requests/admin/ConsumerGroupOffsetsResponse.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/ListTopicsOutput.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/TopicPartitionsDetails.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/requests/admin/VerifyPreferredReplicaLeaderElectionRequest.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/requests/admin/VerifyPreferredReplicaLeaderElectionResponse.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 df37fc6d8f0db0b8192a948426af603be3444da4 config/tools-log4j.properties 52f07c96019b4083fc78f62cfb0a81080327e847 core/src/main/scala/kafka/api/ApiUtils.scala 1f80de1638978901500df808ca5133308c9d1fca core/src/main/scala/kafka/api/ClusterMetadataRequest.scala PRE-CREATION core/src/main/scala/kafka/api/ClusterMetadataResponse.scala PRE-CREATION core/src/main/scala/kafka/api/RequestKeys.scala c24c0345feedc7b9e2e9f40af11bfa1b8d328c43
Re: Review Request 29301: Patch for KAFKA-1694
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29301/ --- (Updated Jan. 12, 2015, 1:28 p.m.) Review request for kafka. Bugs: KAFKA-1694 https://issues.apache.org/jira/browse/KAFKA-1694 Repository: kafka Description (updated) --- 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 Diffs (updated) - bin/kafka.sh PRE-CREATION bin/windows/kafka.bat PRE-CREATION build.gradle ba52288031e2abc70e35e9297a4423dd5025950b clients/src/main/java/org/apache/kafka/common/protocol/ApiKeys.java 109fc965e09b2ed186a073351bd037ac8af20a4c clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 7517b879866fc5dad5f8d8ad30636da8bbe7784a clients/src/main/java/org/apache/kafka/common/protocol/types/MaybeOf.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/protocol/types/Struct.java 121e880a941fcd3e6392859edba11a94236494cc 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/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/ConsumerGroupOffsetsRequest.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/requests/admin/ConsumerGroupOffsetsResponse.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/ListTopicsOutput.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/TopicPartitionsDetails.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/requests/admin/VerifyPreferredReplicaLeaderElectionRequest.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/requests/admin/VerifyPreferredReplicaLeaderElectionResponse.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 df37fc6d8f0db0b8192a948426af603be3444da4 core/src/main/scala/kafka/api/ApiUtils.scala 1f80de1638978901500df808ca5133308c9d1fca core/src/main/scala/kafka/api/ClusterMetadataRequest.scala PRE-CREATION core/src/main/scala/kafka/api/ClusterMetadataResponse.scala PRE-CREATION core/src/main/scala/kafka/api/RequestKeys.scala c24c0345feedc7b9e2e9f40af11bfa1b8d328c43 core/src/main/scala/kafka/api/admin/AlterTopicRequest.scala PRE-CREATION core/src/main/scala/kafka/api/admin/AlterTopicResponse.scala PRE-CREATION core/src/main/scala/kafka/api/admin/ConsumerGroupOffsetsRequest.scala PRE-CREATION core/src/main/scala/kafka/api/admin/ConsumerGroupOffsetsResponse.scala PRE-CREATION
Re: Review Request 29301: Patch for KAFKA-1694
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29301/ --- (Updated Jan. 12, 2015, 4:55 p.m.) Review request for kafka. Bugs: KAFKA-1694 https://issues.apache.org/jira/browse/KAFKA-1694 Repository: kafka Description (updated) --- 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 - kafka-tools is uploaded on uploadAllArchives KAFKA-1694 - ReviewBoard 29301 code review fixes Diffs (updated) - bin/kafka.sh PRE-CREATION bin/windows/kafka.bat PRE-CREATION build.gradle ba52288031e2abc70e35e9297a4423dd5025950b clients/src/main/java/org/apache/kafka/common/protocol/ApiKeys.java 109fc965e09b2ed186a073351bd037ac8af20a4c clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 7517b879866fc5dad5f8d8ad30636da8bbe7784a clients/src/main/java/org/apache/kafka/common/protocol/types/MaybeOf.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/protocol/types/Struct.java 121e880a941fcd3e6392859edba11a94236494cc 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/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/ConsumerGroupOffsetsRequest.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/requests/admin/ConsumerGroupOffsetsResponse.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/ListTopicsOutput.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/TopicPartitionsDetails.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/requests/admin/VerifyPreferredReplicaLeaderElectionRequest.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/requests/admin/VerifyPreferredReplicaLeaderElectionResponse.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 df37fc6d8f0db0b8192a948426af603be3444da4 core/src/main/scala/kafka/api/ApiUtils.scala 1f80de1638978901500df808ca5133308c9d1fca core/src/main/scala/kafka/api/ClusterMetadataRequest.scala PRE-CREATION core/src/main/scala/kafka/api/ClusterMetadataResponse.scala PRE-CREATION core/src/main/scala/kafka/api/RequestKeys.scala c24c0345feedc7b9e2e9f40af11bfa1b8d328c43 core/src/main/scala/kafka/api/admin/AlterTopicRequest.scala PRE-CREATION core/src/main/scala/kafka/api/admin/AlterTopicResponse.scala PRE-CREATION core/src/main/scala/kafka/api/admin/ConsumerGroupOffsetsRequest.scala
Re: Review Request 29301: Patch for KAFKA-1694
On Jan. 6, 2015, 8:19 a.m., Joe Stein wrote: build.gradle, line 211 https://reviews.apache.org/r/29301/diff/2/?file=800278#file800278line211 If we can do this without an upgrade that would be great if we are in fact just requiring 1 function. This feature allows us to call CLI commands both from interactive shell (kafka) and right from ./kafka.sh (e.g. ./kafka.sh--create-topic). The difference is that for ./kafka.sh we always have to supply one additional option --controller which is not recognized during command execution from interactive Shell. As a workaround I can process --controller separately and then simply cut off it from command line args array but this looks a bit lame. On Jan. 6, 2015, 8:19 a.m., Joe Stein wrote: clients/src/main/java/org/apache/kafka/common/protocol/types/MaybeOf.java, line 21 https://reviews.apache.org/r/29301/diff/2/?file=800281#file800281line21 not sure about this name, you are making an Option[] for the protocol value and that not make sense if you look at the use of it and not how it works. As discussed for now better naming option wasn't found:). I'm happy to change if someone comes up with a good one. I also added some comments to the class to make it more explanatory. On Jan. 6, 2015, 8:19 a.m., Joe Stein wrote: clients/src/main/java/org/apache/kafka/common/protocol/types/MaybeOf.java, line 51 https://reviews.apache.org/r/29301/diff/2/?file=800281#file800281line51 why wouldn't the null size == 0? There is always an additional byte showing whether value is present. - Andrii --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29301/#review66801 --- On Jan. 12, 2015, 4:55 p.m., Andrii Biletskyi wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29301/ --- (Updated Jan. 12, 2015, 4:55 p.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 - kafka-tools is uploaded on uploadAllArchives KAFKA-1694 - ReviewBoard 29301 code review fixes Diffs - bin/kafka.sh PRE-CREATION bin/windows/kafka.bat PRE-CREATION build.gradle ba52288031e2abc70e35e9297a4423dd5025950b clients/src/main/java/org/apache/kafka/common/protocol/ApiKeys.java 109fc965e09b2ed186a073351bd037ac8af20a4c clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 7517b879866fc5dad5f8d8ad30636da8bbe7784a clients/src/main/java/org/apache/kafka/common/protocol/types/MaybeOf.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/protocol/types/Struct.java 121e880a941fcd3e6392859edba11a94236494cc 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/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/ConsumerGroupOffsetsRequest.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/requests/admin/ConsumerGroupOffsetsResponse.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/ListTopicsOutput.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/requests/admin/ListTopicsRequest.java
Re: Review Request 29301: Patch for KAFKA-1694
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29301/#review66801 --- build.gradle https://reviews.apache.org/r/29301/#comment110428 We need to have upload archive here build.gradle https://reviews.apache.org/r/29301/#comment110431 If we can do this without an upgrade that would be great if we are in fact just requiring 1 function. build.gradle https://reviews.apache.org/r/29301/#comment110432 ??? clients/src/main/java/org/apache/kafka/common/protocol/types/MaybeOf.java https://reviews.apache.org/r/29301/#comment110437 not sure about this name, you are making an Option[] for the protocol value and that not make sense if you look at the use of it and not how it works. clients/src/main/java/org/apache/kafka/common/protocol/types/MaybeOf.java https://reviews.apache.org/r/29301/#comment110439 why wouldn't the null size == 0? core/src/main/scala/kafka/common/ErrorMapping.scala https://reviews.apache.org/r/29301/#comment110440 This is for trying to send an admin request to a broker that is not the controller? I think it should be name for that more specifically and clearly/ - Joe Stein On Dec. 24, 2014, 7:22 p.m., Andrii Biletskyi wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29301/ --- (Updated Dec. 24, 2014, 7:22 p.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 Diffs - bin/kafka.sh PRE-CREATION bin/windows/kafka.bat PRE-CREATION build.gradle 18f86e4c8a10618d50ac78572d119c6e100ed85b clients/src/main/java/org/apache/kafka/common/protocol/ApiKeys.java 109fc965e09b2ed186a073351bd037ac8af20a4c clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 7517b879866fc5dad5f8d8ad30636da8bbe7784a clients/src/main/java/org/apache/kafka/common/protocol/types/MaybeOf.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/protocol/types/Struct.java 121e880a941fcd3e6392859edba11a94236494cc 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/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/ListTopicsOutput.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/TopicConfigDetails.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/requests/admin/TopicPartitionsDetails.java PRE-CREATION clients/src/test/java/org/apache/kafka/common/requests/RequestResponseTest.java df37fc6d8f0db0b8192a948426af603be3444da4 core/src/main/scala/kafka/api/ApiUtils.scala 1f80de1638978901500df808ca5133308c9d1fca core/src/main/scala/kafka/api/ClusterMetadataRequest.scala PRE-CREATION core/src/main/scala/kafka/api/ClusterMetadataResponse.scala PRE-CREATION core/src/main/scala/kafka/api/RequestKeys.scala c24c0345feedc7b9e2e9f40af11bfa1b8d328c43 core/src/main/scala/kafka/api/admin/AlterTopicRequest.scala PRE-CREATION core/src/main/scala/kafka/api/admin/AlterTopicResponse.scala PRE-CREATION
Re: Review Request 29301: Patch for KAFKA-1694
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29301/#review66838 --- tools/src/main/java/org/apache/kafka/cli/command/AlterTopicCommand.java https://reviews.apache.org/r/29301/#comment110502 minor nit: grammar. The topic to be created, altered, or described - Jeff Holoman On Dec. 24, 2014, 7:22 p.m., Andrii Biletskyi wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29301/ --- (Updated Dec. 24, 2014, 7:22 p.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 Diffs - bin/kafka.sh PRE-CREATION bin/windows/kafka.bat PRE-CREATION build.gradle 18f86e4c8a10618d50ac78572d119c6e100ed85b clients/src/main/java/org/apache/kafka/common/protocol/ApiKeys.java 109fc965e09b2ed186a073351bd037ac8af20a4c clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 7517b879866fc5dad5f8d8ad30636da8bbe7784a clients/src/main/java/org/apache/kafka/common/protocol/types/MaybeOf.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/protocol/types/Struct.java 121e880a941fcd3e6392859edba11a94236494cc 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/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/ListTopicsOutput.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/TopicConfigDetails.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/requests/admin/TopicPartitionsDetails.java PRE-CREATION clients/src/test/java/org/apache/kafka/common/requests/RequestResponseTest.java df37fc6d8f0db0b8192a948426af603be3444da4 core/src/main/scala/kafka/api/ApiUtils.scala 1f80de1638978901500df808ca5133308c9d1fca core/src/main/scala/kafka/api/ClusterMetadataRequest.scala PRE-CREATION core/src/main/scala/kafka/api/ClusterMetadataResponse.scala PRE-CREATION core/src/main/scala/kafka/api/RequestKeys.scala c24c0345feedc7b9e2e9f40af11bfa1b8d328c43 core/src/main/scala/kafka/api/admin/AlterTopicRequest.scala PRE-CREATION core/src/main/scala/kafka/api/admin/AlterTopicResponse.scala PRE-CREATION core/src/main/scala/kafka/api/admin/CreateTopicRequest.scala PRE-CREATION core/src/main/scala/kafka/api/admin/CreateTopicResponse.scala PRE-CREATION core/src/main/scala/kafka/api/admin/DeleteTopicRequest.scala PRE-CREATION core/src/main/scala/kafka/api/admin/DeleteTopicResponse.scala PRE-CREATION core/src/main/scala/kafka/api/admin/DescribeTopicRequest.scala PRE-CREATION core/src/main/scala/kafka/api/admin/DescribeTopicResponse.scala PRE-CREATION core/src/main/scala/kafka/api/admin/ListTopicsRequest.scala PRE-CREATION core/src/main/scala/kafka/api/admin/ListTopicsResponse.scala PRE-CREATION core/src/main/scala/kafka/common/AdminRequestFailedException.scala PRE-CREATION core/src/main/scala/kafka/common/ErrorMapping.scala eedc2f5f21dd8755fba891998456351622e17047 core/src/main/scala/kafka/common/InvalidRequestTargetException.scala PRE-CREATION
Re: Review Request 29301: Patch for KAFKA-1694
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29301/ --- (Updated Dec. 24, 2014, 7:22 p.m.) Review request for kafka. Bugs: KAFKA-1694 https://issues.apache.org/jira/browse/KAFKA-1694 Repository: kafka Description (updated) --- 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 Diffs (updated) - bin/kafka.sh PRE-CREATION bin/windows/kafka.bat PRE-CREATION build.gradle 18f86e4c8a10618d50ac78572d119c6e100ed85b clients/src/main/java/org/apache/kafka/common/protocol/ApiKeys.java 109fc965e09b2ed186a073351bd037ac8af20a4c clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 7517b879866fc5dad5f8d8ad30636da8bbe7784a clients/src/main/java/org/apache/kafka/common/protocol/types/MaybeOf.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/protocol/types/Struct.java 121e880a941fcd3e6392859edba11a94236494cc 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/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/ListTopicsOutput.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/TopicConfigDetails.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/requests/admin/TopicPartitionsDetails.java PRE-CREATION clients/src/test/java/org/apache/kafka/common/requests/RequestResponseTest.java df37fc6d8f0db0b8192a948426af603be3444da4 core/src/main/scala/kafka/api/ApiUtils.scala 1f80de1638978901500df808ca5133308c9d1fca core/src/main/scala/kafka/api/ClusterMetadataRequest.scala PRE-CREATION core/src/main/scala/kafka/api/ClusterMetadataResponse.scala PRE-CREATION core/src/main/scala/kafka/api/RequestKeys.scala c24c0345feedc7b9e2e9f40af11bfa1b8d328c43 core/src/main/scala/kafka/api/admin/AlterTopicRequest.scala PRE-CREATION core/src/main/scala/kafka/api/admin/AlterTopicResponse.scala PRE-CREATION core/src/main/scala/kafka/api/admin/CreateTopicRequest.scala PRE-CREATION core/src/main/scala/kafka/api/admin/CreateTopicResponse.scala PRE-CREATION core/src/main/scala/kafka/api/admin/DeleteTopicRequest.scala PRE-CREATION core/src/main/scala/kafka/api/admin/DeleteTopicResponse.scala PRE-CREATION core/src/main/scala/kafka/api/admin/DescribeTopicRequest.scala PRE-CREATION core/src/main/scala/kafka/api/admin/DescribeTopicResponse.scala PRE-CREATION core/src/main/scala/kafka/api/admin/ListTopicsRequest.scala PRE-CREATION core/src/main/scala/kafka/api/admin/ListTopicsResponse.scala PRE-CREATION core/src/main/scala/kafka/common/AdminRequestFailedException.scala PRE-CREATION core/src/main/scala/kafka/common/ErrorMapping.scala eedc2f5f21dd8755fba891998456351622e17047 core/src/main/scala/kafka/common/InvalidRequestTargetException.scala PRE-CREATION core/src/main/scala/kafka/controller/ControllerChannelManager.scala eb492f00449744bc8d63f55b393e2a1659d38454 core/src/main/scala/kafka/controller/KafkaController.scala 66df6d2fbdbdd556da6bea0df84f93e0472c8fbf core/src/main/scala/kafka/server/KafkaApis.scala 2a1c0326b6e6966d8b8254bd6a1cb83ad98a3b80 core/src/main/scala/kafka/server/MetadataCache.scala bf81a1ab88c14be8697b441eedbeb28fa0112643 core/src/main/scala/kafka/server/TopicCommandHelper.scala PRE-CREATION core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala cd16ced5465d098be7a60498326b2a98c248f343
Review Request 29301: Patch for KAFKA-1694
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29301/ --- 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 Diffs - bin/kafka.sh PRE-CREATION bin/windows/kafka.bat PRE-CREATION build.gradle 18f86e4c8a10618d50ac78572d119c6e100ed85b clients/src/main/java/org/apache/kafka/common/protocol/ApiKeys.java 109fc965e09b2ed186a073351bd037ac8af20a4c clients/src/main/java/org/apache/kafka/common/protocol/Protocol.java 7517b879866fc5dad5f8d8ad30636da8bbe7784a clients/src/main/java/org/apache/kafka/common/protocol/types/MaybeOf.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/protocol/types/Struct.java 121e880a941fcd3e6392859edba11a94236494cc 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/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/ListTopicsOutput.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/TopicConfigDetails.java PRE-CREATION clients/src/main/java/org/apache/kafka/common/requests/admin/TopicPartitionsDetails.java PRE-CREATION clients/src/test/java/org/apache/kafka/common/requests/RequestResponseTest.java df37fc6d8f0db0b8192a948426af603be3444da4 core/src/main/scala/kafka/api/ApiUtils.scala 1f80de1638978901500df808ca5133308c9d1fca core/src/main/scala/kafka/api/ClusterMetadataRequest.scala PRE-CREATION core/src/main/scala/kafka/api/ClusterMetadataResponse.scala PRE-CREATION core/src/main/scala/kafka/api/RequestKeys.scala c24c0345feedc7b9e2e9f40af11bfa1b8d328c43 core/src/main/scala/kafka/api/admin/AlterTopicRequest.scala PRE-CREATION core/src/main/scala/kafka/api/admin/AlterTopicResponse.scala PRE-CREATION core/src/main/scala/kafka/api/admin/CreateTopicRequest.scala PRE-CREATION core/src/main/scala/kafka/api/admin/CreateTopicResponse.scala PRE-CREATION core/src/main/scala/kafka/api/admin/DeleteTopicRequest.scala PRE-CREATION core/src/main/scala/kafka/api/admin/DeleteTopicResponse.scala PRE-CREATION core/src/main/scala/kafka/api/admin/DescribeTopicRequest.scala PRE-CREATION core/src/main/scala/kafka/api/admin/DescribeTopicResponse.scala PRE-CREATION core/src/main/scala/kafka/api/admin/ListTopicsRequest.scala PRE-CREATION core/src/main/scala/kafka/api/admin/ListTopicsResponse.scala PRE-CREATION core/src/main/scala/kafka/common/AdminRequestFailedException.scala PRE-CREATION core/src/main/scala/kafka/common/ErrorMapping.scala eedc2f5f21dd8755fba891998456351622e17047 core/src/main/scala/kafka/common/InvalidRequestTargetException.scala PRE-CREATION core/src/main/scala/kafka/controller/ControllerChannelManager.scala eb492f00449744bc8d63f55b393e2a1659d38454 core/src/main/scala/kafka/controller/KafkaController.scala 66df6d2fbdbdd556da6bea0df84f93e0472c8fbf core/src/main/scala/kafka/server/KafkaApis.scala 2a1c0326b6e6966d8b8254bd6a1cb83ad98a3b80 core/src/main/scala/kafka/server/MetadataCache.scala bf81a1ab88c14be8697b441eedbeb28fa0112643 core/src/main/scala/kafka/server/TopicCommandHelper.scala PRE-CREATION core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala cd16ced5465d098be7a60498326b2a98c248f343 settings.gradle 83f764e6a4a15a5fdba232dce74a369870f26b45 tools/src/main/java/org/apache/kafka/cli/BaseCommandOpts.java PRE-CREATION
