Yeah I totally agree that we don't want to just have one "do admin stuff" command that has the union of all parameters.
What I am saying is that command line tools are one client of the administrative apis, but these will be used in a number of scenarios so they should make logical sense even in the absence of the command line tool. Hence comments like trying to clarify the relationship between ClusterMetadata and TopicMetadata...these kinds of things really need to be thought through. Hope that makes sense. -Jay On Wed, Feb 11, 2015 at 1:41 PM, Andrii Biletskyi < andrii.bilets...@stealth.ly> wrote: > 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 > > > > > > > ********************************************/ > > > > > > > > > > > > > > >
