Hey Andrii,
Generally we can do good error handling without needing custom server-side
messages. I.e. generally the client has the context to know that if it got
an error that the topic doesn't exist to say "Topic X doesn't exist" rather
than "error code 14" (or whatever). Maybe there are specific cases where
this is hard? If we want to add server-side error messages we really do
need to do this in a consistent way across the protocol.
I still have a bunch of open questions here from my previous list. I will
be out for the next few days for Strata though. Maybe we could do a Google
Hangout chat on any open issues some time towards the end of next week for
anyone interested in this ticket? I have a feeling that might progress
things a little faster than email--I think we could talk through those
issues I brought up fairly quickly...
-Jay
On Wed, Feb 18, 2015 at 7:27 AM, Andrii Biletskyi <
andrii.bilets...@stealth.ly> wrote:
> Hi all,
>
> I'm trying to address some of the issues which were mentioned earlier about
> Admin RQ/RP format. One of those was about batching operations. What if we
> follow TopicCommand approach and let people specify topic-name by regexp -
> would that cover most of the use cases?
>
> Secondly, is what information should we generally provide in Admin
> responses.
> I realize that Admin commands don't imply they will be used only in CLI
> but,
> it seems to me, CLI is a very important client of this feature. In this
> case,
> seems logical, we would like to provide users with rich experience in terms
> of
> getting results / errors of the executed commands. Usually we supply with
> responses only errorCode, which looks very limiting, in case of CLI we may
> want to print human readable error description.
>
> So, taking into account previous item about batching, what do you think
> about
> having smth like:
>
> ('create' doesn't support regexp)
> CreateTopicRequest => TopicName Partitions Replicas ReplicaAssignment
> [Config]
> CreateTopicResponse => ErrorCode ErrorDescription
> ErrorCode => int16
> ErrorDescription => string (empty if successful)
>
> AlterTopicRequest -> TopicNameRegexp Partitions ReplicaAssignment
> [AddedConfig] [DeletedConfig]
> AlterTopicResponse -> [TopicName ErrorCode ErrorDescription]
> CommandErrorCode CommandErrorDescription
> CommandErrorCode => int16
> CommandErrorDescription => string (nonempty in case of fatal error, e.g.
> we couldn't get topics by regexp)
>
> DescribeTopicRequest -> TopicNameRegexp
> DescribeTopicResponse -> [TopicName TopicDescription ErrorCode
> ErrorDescription] CommandErrorCode CommandErrorDescription
>
> Also, any thoughts about our discussion regarding re-routing facility? In
> my
> understanding, it is like between augmenting TopicMetadataRequest
> (to include at least controllerId) and implementing new generic re-routing
> facility so sending messages to controller will be handled by it.
>
> Thanks,
> Andrii Biletskyi
>
> On Mon, Feb 16, 2015 at 5:26 PM, Andrii Biletskyi <
> andrii.bilets...@stealth.ly> wrote:
>
> > @Guozhang:
> > Thanks for your comments, I've answered some of those. The main thing is
> > having merged request for create-alter-delete-describe - I have some
> > concerns about this approach.
> >
> > @*Jay*:
> > I see that introduced ClusterMetadaRequest is also one of the concerns.
> We
> > can solve it if we implement re-routing facility. But I agree with
> > Guozhang - it will make clients' internals a little bit easier but this
> > seems to be a complex logic to implement and support then. Especially for
> > Fetch and Produce (even if we add re-routing later for these requests).
> > Also people will tend to avoid this re-routing facility and hold local
> > cluster cache to ensure their high-priority requests (which some of the
> > admin requests are) not sent to some busy broker where they wait to be
> > routed to the correct one.
> > As pointed out by Jun here (
> >
> https://issues.apache.org/jira/browse/KAFKA-1772?focusedCommentId=14234530&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14234530
> )
> > to solve the issue we might introduce a message type to get cluster
> state.
> > But I agree we can just update TopicMetadataResponse to include
> > controllerId (and probably smth else).
> > What are you thougths?
> >
> > Thanks,
> > Andrii
> >
> > On Thu, Feb 12, 2015 at 8:31 AM, Guozhang Wang <wangg...@gmail.com>
> wrote:
> >
> >> I think for the topics commands we can actually merge
> >> create/alter/delete/describe as one request type since their formats are
> >> very much similar, and keep list-topics and others like
> >> partition-reassignment / preferred-leader-election as separate request
> >> types, I also left some other comments on the RB (
> >> https://reviews.apache.org/r/29301/).
> >>
> >> On Wed, Feb 11, 2015 at 2:04 PM, Jay Kreps <jay.kr...@gmail.com> wrote:
> >>
> >> > 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
> >> > > >
> >> > > > > > > ********************************************/
> >> > > > > > >
> >> > > > > >
> >> > > > >
> >> > > >
> >> > >
> >> >
> >>
> >>
> >>
> >> --
> >> -- Guozhang
> >>
> >
> >
>
