Jay,
Re error messages: you are right, in most cases client will have enough
context to show descriptive error message. My concern is that we will have
to
add lots of new error codes for each possible error. Of course, we could
reuse
some of existing like UknownTopicOrPartitionCode, but we will also need to
add smth like: TopicAlreadyExistsCode, TopicConfigInvalid (both for topic
name and config, and probably user would like to know what exactly
is wrong in his config), InvalidReplicaAssignment, InternalError (e.g.
zookeeper failure) etc.
And this is only for TopicCommand, we will also need to add similar stuff
for
ReassignPartitions, PreferredReplica. So we'll end up with a large list of
error codes, used only in Admin protocol.
Having said that, I agree my proposal is not consistent with other cases.
Maybe we can find better solution or something in-between.
Re Hangout chat: I think it is a great idea. This way we can move on faster.
Let's agree somehow on date/time so people can join. Will work for me this
and
next week almost anytime if agreed in advance.
Thanks,
Andrii
On Wed, Feb 18, 2015 at 7:09 PM, Jay Kreps <jay.kr...@gmail.com> wrote:
> 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
> > >>
> > >
> > >
> >
>
