Grant, Thanks for the writeup. A few comments.
1. It seems a bit weird to return just a list of internal topics w/o the corresponding metadata. It also seems a bit weird to return the internal topics even if the client doesn't ask for it. Would it be better to just add a flag in topic_metadata to indicate whether it's an internal topic or not, and only include the internal topics when thy are asked (or all topics are requested) for? 2. A similar comment on topics_marked_for_deletion. Would it be better to only return them when asked for and just return a new TopicDeleted error code in topic_metadata? Implementation wise, it seems that you propagated the topic deletion marker by having the replicaManager read from ZK directly. It seems that it would be simpler/consistent if the controller propagates that information directly through UpdateMetaRequest. 3. Could you add a description on how controller id will be used in the client? 4. We had a weird semantic in version 0 of MetadataRequest. If a replica is not live, but the leader is live, we return an error ReplicaNotAvailableException in the partition metadata. This makes it a bit confusing for the client to parse since it has to first check whether leader is available or not before error code checking. We were thinking of changing that behavior the next time we bump up the version of MetadataRequest. Now that time has come, could you include that in the proposal? Thanks, Jun On Fri, Apr 1, 2016 at 8:19 AM, Grant Henke <ghe...@cloudera.com> wrote: > I would like to start the voting process for the "KIP-4 Metadata Schema > changes". This is not a vote for all of KIP-4, but specifically for the > metadata changes. I have included the exact changes below for clarity: > > > Metadata Request (version 1) > > > > > > > > MetadataRequest => [topics] > > > > Stays the same as version 0 however behavior changes. > > In version 0 there was no way to request no topics, and and empty list > > signified all topics. > > In version 1 a null topics list (size -1 on the wire) will indicate that > a > > user wants *ALL* topic metadata. Compared to an empty list (size 0) which > > indicates metadata for *NO* topics should be returned. > > Metadata Response (version 1) > > > > > > > > MetadataResponse => [brokers] controller_id [internal_topics] > [topics_marked_for_deletion] [topic_metadata] > > brokers => node_id host port rack > > node_id => INT32 > > host => STRING > > port => INT32 > > rack => NULLABLE_STRING > > controller_id => INT32 > > internal_topics => STRING > > topics_marked_for_deletion => STRING > > topic_metadata => topic_error_code topic [partition_metadata] > > topic_error_code => INT16 > > topic => STRING > > partition_metadata => partition_error_code partition_id leader > [replicas] [isr] > > partition_error_code => INT16 > > partition_id => INT32 > > leader => INT32 > > > > Adds rack, controller_id, internal_topics and topics_marked_for_deletion > > to the version 0 response. > > > > The KIP is available here for reference (linked to the Metadata schema > section): > * > https://cwiki.apache.org/confluence/display/KAFKA/KIP-4+-+Command+line+and+centralized+administrative+operations#KIP-4-Commandlineandcentralizedadministrativeoperations-MetadataSchema > < > https://cwiki.apache.org/confluence/display/KAFKA/KIP-4+-+Command+line+and+centralized+administrative+operations#KIP-4-Commandlineandcentralizedadministrativeoperations-MetadataSchema > >* > > A pull request is available implementing the proposed changes here: > https://github.com/apache/kafka/pull/1095 > > Here are some links to past discussions on the mailing list: > http://search-hadoop.com/m/uyzND1pd4T52H1m0u1&subj=Re+KIP+4+Wiki+Update > > http://search-hadoop.com/m/uyzND1J2IXeSNXAT&subj=Metadata+and+ACLs+wire+protocol+review+KIP+4+ > > Thank you, > Grant > -- > Grant Henke > Software Engineer | Cloudera > gr...@cloudera.com | twitter.com/gchenke | linkedin.com/in/granthenke >
