Jay, Thanks for answering. You understood correctly, most of my comments were related to your point 1) - about "well thought-out" apis. Also, yes, as I understood we would like to introduce a single unified CLI tool with centralized server-side request handling for lots of existing ones (incl. TopicCommand, CommitOffsetChecker, ReassignPartitions, smth else if added in future). In our previous discussion ( https://issues.apache.org/jira/browse/KAFKA-1694) people said they'd rather have a separate message for each command, so, yes, this way I came to 1-1 mapping between commands in the tool and protocol additions. But I might be wrong. At the end I just try to start discussion how at least generally this protocol should look like.
Thanks, Andrii On Wed, Feb 11, 2015 at 11:10 PM, Jay Kreps <jay.kr...@gmail.com> wrote: > Hey Andrii, > > To answer your earlier question we just really can't be adding any more > scala protocol objects. These things are super hard to maintain because > they hand code the byte parsing and don't have good versioning support. > Since we are already planning on converting we definitely don't want to add > a ton more of these--they are total tech debt. > > What does it mean that the changes are isolated from the current code base? > > I actually didn't understand the remaining comments, which of the points > are you responding to? > > Maybe one sticking point here is that it seems like you want to make some > kind of tool, and you have made a 1-1 mapping between commands you imagine > in the tool and protocol additions. I want to make sure we don't do that. > The protocol needs to be really really well thought out against many use > cases so it should make perfect logical sense in the absence of knowing the > command line tool, right? > > -Jay > > On Wed, Feb 11, 2015 at 11:57 AM, Andrii Biletskyi < > andrii.bilets...@stealth.ly> wrote: > > > Hey Jay, > > > > I would like to continue this discussion as it seem there is no progress > > here. > > > > First of all, could you please explain what did you mean in 2? How > exactly > > are we going to migrate to the new java protocol definitions. And why > it's > > a blocker for centralized CLI? > > > > I agree with you, this feature includes lots of stuff, but thankfully > > almost all changes are isolated from the current code base, > > so the main thing, I think, we need to agree is RQ/RP format. > > So how can we start discussion about the concrete messages format? > > Can we take ( > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-4+-+Command+line+and+centralized+administrative+operations#KIP-4-Commandlineandcentralizedadministrativeoperations-ProposedRQ/RPFormat > > ) > > as starting point? > > > > We had some doubts earlier whether it worth introducing one generic Admin > > Request for all commands ( > https://issues.apache.org/jira/browse/KAFKA-1694 > > ) > > but then everybody agreed it would be better to have separate message for > > each admin command. The Request part is really dictated from the command > > (e.g. TopicCommand) arguments itself, so the proposed version should be > > fine (let's put aside for now remarks about Optional type, batching, > > configs normalization - I agree with all of them). > > So the second part is Response. I see there are two cases here. > > a) "Mutate" requests - Create/Alter/... ; b) "Get" requests - > > List/Describe... > > > > a) should only hold request result (regardless what we decide about > > blocking/non-blocking commands execution). > > Usually we provide error code in response but since we will use this in > > interactive shell we need some human readable error description - so I > > added errorDesription field where you can at least leave > > exception.getMessage. > > > > b) in addition to previous item message should hold command specific > > response data. We can discuss in detail each of them but let's for now > > agree about the overall pattern. > > > > Thanks, > > Andrii Biletskyi > > > > On Fri, Jan 23, 2015 at 6:59 AM, Jay Kreps <jay.kr...@gmail.com> wrote: > > > > > Hey Joe, > > > > > > This is great. A few comments on KIP-4 > > > > > > 1. This is much needed functionality, but there are a lot of the so > let's > > > really think these protocols through. We really want to end up with a > set > > > of well thought-out, orthoganol apis. For this reason I think it is > > really > > > important to think through the end state even if that includes APIs we > > > won't implement in the first phase. > > > > > > 2. Let's please please please wait until we have switched the server > over > > > to the new java protocol definitions. If we add upteen more ad hoc > scala > > > objects that is just generating more work for the conversion we know we > > > have to do. > > > > > > 3. This proposal introduces a new type of optional parameter. This is > > > inconsistent with everything else in the protocol where we use -1 or > some > > > other marker value. You could argue either way but let's stick with > that > > > for consistency. For clients that implemented the protocol in a better > > way > > > than our scala code these basic primitives are hard to change. > > > > > > 4. ClusterMetadata: This seems to duplicate TopicMetadataRequest which > > has > > > brokers, topics, and partitions. I think we should rename that request > > > ClusterMetadataRequest (or just MetadataRequest) and include the id of > > the > > > controller. Or are there other things we could add here? > > > > > > 5. We have a tendency to try to make a lot of requests that can only go > > to > > > particular nodes. This adds a lot of burden for client implementations > > (it > > > sounds easy but each discovery can fail in many parts so it ends up > > being a > > > full state machine to do right). I think we should consider making > admin > > > commands and ideally as many of the other apis as possible available on > > all > > > brokers and just redirect to the controller on the broker side. Perhaps > > > there would be a general way to encapsulate this re-routing behavior. > > > > > > 6. We should probably normalize the key value pairs used for configs > > rather > > > than embedding a new formatting. So two strings rather than one with an > > > internal equals sign. > > > > > > 7. Is the postcondition of these APIs that the command has begun or > that > > > the command has been completed? It is a lot more usable if the command > > has > > > been completed so you know that if you create a topic and then publish > to > > > it you won't get an exception about there being no such topic. > > > > > > 8. Describe topic and list topics duplicate a lot of stuff in the > > metadata > > > request. Is there a reason to give back topics marked for deletion? I > > feel > > > like if we just make the post-condition of the delete command be that > the > > > topic is deleted that will get rid of the need for this right? And it > > will > > > be much more intuitive. > > > > > > 9. Should we consider batching these requests? We have generally tried > to > > > allow multiple operations to be batched. My suspicion is that without > > this > > > we will get a lot of code that does something like > > > for(topic: adminClient.listTopics()) > > > adminClient.describeTopic(topic) > > > this code will work great when you test on 5 topics but not do as well > if > > > you have 50k. > > > > > > 10. I think we should also discuss how we want to expose a programmatic > > JVM > > > client api for these operations. Currently people rely on AdminUtils > > which > > > is totally sketchy. I think we probably need another client under > > clients/ > > > that exposes administrative functionality. We will need this just to > > > properly test the new apis, I suspect. We should figure out that API. > > > > > > 11. The other information that would be really useful to get would be > > > information about partitions--how much data is in the partition, what > are > > > the segment offsets, what is the log-end offset (i.e. last offset), > what > > is > > > the compaction point, etc. I think that done right this would be the > > > successor to the very awkward OffsetRequest we have today. > > > > > > -Jay > > > > > > On Wed, Jan 21, 2015 at 10:27 PM, Joe Stein <joe.st...@stealth.ly> > > wrote: > > > > > > > Hi, created a KIP > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-4+-+Command+line+and+centralized+administrative+operations > > > > > > > > JIRA https://issues.apache.org/jira/browse/KAFKA-1694 > > > > > > > > /******************************************* > > > > Joe Stein > > > > Founder, Principal Consultant > > > > Big Data Open Source Security LLC > > > > http://www.stealth.ly > > > > Twitter: @allthingshadoop <http://www.twitter.com/allthingshadoop> > > > > ********************************************/ > > > > > > > > > >
