Re: [DISCUSS] KIP-4 Create Topic Schema

For what is worth, I also agree that this is better. It is awkward to implement and it is not done by any other request and that is why I didn't push for it, but happy that we are going in that direction. It's worth noting that even with this change, it seems to me that looking at the logs may stil

Re: [DISCUSS] KIP-4 Create Topic Schema

Thanks Grant. Sorry to make the handling on the broker more complicated :( but it seems like the right way to handle this. -Ewen On Sun, Jun 19, 2016 at 8:57 PM, Grant Henke wrote: > Apologies for the delay in response here. > > It will take a bit of tracking inside the request object to track

Re: [DISCUSS] KIP-4 Create Topic Schema

Apologies for the delay in response here. It will take a bit of tracking inside the request object to track this error and then handle it in KafkaApis, but it is possible. I am happy to take that preferred approach. I will update the wiki & patch to handle this scenario and re-initiate the vote to

Re: [DISCUSS] KIP-4 Create Topic Schema

I'm on the same page as Jun & Dana wrt disconnecting. Closing a connection should really be a last resort because we can no longer trust correct behavior in this session. In this case, we detect a bad request, but there's no reason to believe it will affect subsequent requests. There are dependenci

Re: [DISCUSS] KIP-4 Create Topic Schema

Grant, I think Dana has a valid point. Currently, we throw an InvalidRequestException and close the connection only when the broker can't deserialize the bytes into a request. In this case, the deserialization is fine. It just that there are some additional constraints that can't be specified at t

Re: [DISCUSS] KIP-4 Create Topic Schema

I'm unconvinced (crazy, right?). Comments below: On Fri, Jun 17, 2016 at 7:27 AM, Grant Henke wrote: > Hi Dana, > > You mentioned one of the reasons I error and disconnect. Because I can't > return an error for every request so the cardinality between request and > response would be different. Be

Re: [DISCUSS] KIP-4 Create Topic Schema

Hi Dana, You mentioned one of the reasons I error and disconnect. Because I can't return an error for every request so the cardinality between request and response would be different. Beyond that though, I am handling this protocol rule/parsing error the same way all other messages do. Parsing is

Re: [DISCUSS] KIP-4 Create Topic Schema

Why disconnect the client on a InvalidRequestException? The 2 errors you are catching are both topic-level: (1) multiple requests for the same topic, and (2) ReplicaAssignment and num_partitions / replication_factor both set. Wouldn't it be better to just error the offending create_topic_request, n

Re: [DISCUSS] KIP-4 Create Topic Schema

I have updated the wiki and pull request based on the feedback. If there are no objections I will start a vote at the end of the day. Details for this implementation can be read here: https://cwiki.apache.org/confluence/display/KAFKA/KIP-4+-+Command+line+and+centralized+administrative+operations#K

Re: [DISCUSS] KIP-4 Create Topic Schema

Turns out we already have an InvalidRequestException: https://github.com/apache/kafka/blob/trunk/core/src/main/scala/kafka/network/RequestChannel.scala#L75-L98 We just don't map it in Errors.java so it results in an UNKNOWN error when sent back to the client. I will migrate the InvalidRequestExce

Re: [DISCUSS] KIP-4 Create Topic Schema

On Wed, Jun 15, 2016 at 12:19 PM, Ismael Juma wrote: > Hi Grant, > > Comments below. > > On Wed, Jun 15, 2016 at 6:52 PM, Grant Henke wrote: >> >> The one thing I want to avoid is to many super specific error codes. I am >> not sure how much of a problem it really is but in the case of wire >> pr

Re: [DISCUSS] KIP-4 Create Topic Schema

Hi Grant, Comments below. On Wed, Jun 15, 2016 at 6:52 PM, Grant Henke wrote: > > The one thing I want to avoid is to many super specific error codes. I am > not sure how much of a problem it really is but in the case of wire > protocol errors like multiple instances of the same topic, do you ha

Re: [DISCUSS] KIP-4 Create Topic Schema

Thanks for the feedback Ewen & Ismael. Fine about keeping replication_factor as INT32 (you mentioned 32k > partitions in your reply, but I was talking about replication_factor which > doesn't seem feasible to ever be that large. I didn't mention > num_partitions for the reason you mentioned). My

Re: [DISCUSS] KIP-4 Create Topic Schema

> Ewen's reply sums up my thoughts on the error handling points. It doesn't > seem ideal to justify the wire protocol behaviour based on the Java > implementation. If we need map-like semantics in the protocol, then maybe > we need a `Map` type to complement `Array`? Otherwise, I still think we > s

Re: [DISCUSS] KIP-4 Create Topic Schema

Thanks for the responses Grant. Fine about keeping replication_factor as INT32 (you mentioned 32k partitions in your reply, but I was talking about replication_factor which doesn't seem feasible to ever be that large. I didn't mention num_partitions for the reason you mentioned). Ewen's reply sum

Re: [DISCUSS] KIP-4 Create Topic Schema

I would second Ismael's points on 2 & 4. Sometimes its good to be liberal with the input you accept, but I don't think that applies here -- there are specific rules and if the user gives inconsistent input that indicates, I don't think we should just hope that prioritizing one arbitrarily (the map)

Re: [DISCUSS] KIP-4 Create Topic Schema

Thanks for the review Ismael. `partition_count` or `num_partitions` seems clearer to me. Agreed, I updated the wiki and patch to use num_partitions. I wondered if this should be `INT16`. Maybe not worth it as it won't make > much of a difference in terms of the request size though. > Since In

Re: [DISCUSS] KIP-4 Create Topic Schema

Hi Grant, Thanks for the proposal. A few comments and questions below. On Fri, Jun 10, 2016 at 6:21 PM, Grant Henke wrote: > > CreateTopic Request (Version: 0) => [create_topic_requests] timeout > > create_topic_requests => topic partitions replication_factor > [replica_assignment] [configs]

Re: [DISCUSS] KIP-4 Create Topic Schema

Hi Guozhang, Thanks for the review. I listed the possible CreateTopic Response error codes in my patch. Below are the relevant error codes and how I think the admin client or producer would handle them. This can be vetted more thoroughly in the code reviews: - INVALID_TOPIC_EXCEPTION(17) -

Re: [DISCUSS] KIP-4 Create Topic Schema

Thanks Grant, the design proposal LGTM overall. One minor question about the error codes in CreateTopic Response, what are the possible values? I know this may be out of the scope of this KIP, but would also want to think about how producers should handle each one of them accordingly, especially i

Re: [DISCUSS] KIP-4 Create Topic Schema

Thanks for the review Jun. You probably want to make it clearer if timeout > 0, what waiting for topic > metadata is "complete" means. In the first implementation, it really means > that the topic metadata is propagated to the controller's metadata cache. I updated the wiki to be more descriptiv

Re: [DISCUSS] KIP-4 Create Topic Schema

Thanks for the review Gwen. 1. The replica assignment protocol takes [replicas], there is the > implicit assumption that the first replica is the leader. This matches > current behavior elsewhere, but lets document it explicitly. I added this to the wiki and will update the protocol doc string i

Re: [DISCUSS] KIP-4 Create Topic Schema

Hi Jay, Good point one of the main benefits of the create topic api is removing the server side auto create. The work is noted in the Follow Up Changes

Re: [DISCUSS] KIP-4 Create Topic Schema

Grant, Thanks for the proposal. It looks good to me. You probably want to make it clearer if timeout > 0, what waiting for topic metadata is "complete" means. In the first implementation, it really means that the topic metadata is propagated to the controller's metadata cache. Jun On Fri, Jun 1

Re: [DISCUSS] KIP-4 Create Topic Schema

Hey Grant, Great to see this progressing. That API looks good to me. Thanks for the thoughtful write-up. One thing that would be great to add to this KIP would be a quick sketch of how the create topic api can be used to get rid of the thing where we create topics when you ask for their metadata.

Re: [DISCUSS] KIP-4 Create Topic Schema

Thank you for the clear proposal, Grant! I like the request/response objects and the timeout semantics. Two comments: 1. The replica assignment protocol takes [replicas], there is the implicit assumption that the first replica is the leader. This matches current behavior elsewhere, but lets docum

[DISCUSS] KIP-4 Create Topic Schema

Now that Kafka 0.10 has been released I would like to start work on the new protocol messages and client implementation for KIP-4. In order to break up the discussion and feedback I would like to continue breaking up the content in to smaller pieces. This discussion thread is for the CreateTopic r