kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ron Dagostino <rndg...@gmail.com>
Subject Re: [DISCUSS] KIP-631: The Quorum-based Kafka Controller
Date Wed, 02 Dec 2020 22:07:11 GMT
Hi Colin.  Thanks for the updates.  It's now clear to me that brokers
keep their broker epoch for the life of their JVM -- they register
once, get their broker epoch in the response, and then never
re-register again.  Brokers may get fenced, but they keep the same
broker epoch for the life of their JVM.  The incarnation ID is also
kept for the life of the JVM but is generated by the broker itself
upon startup, and the combination of the two allows the Controller to
act idempotently if any previously-sent registration response gets
lost.  Makes sense.

One thing I wonder about is if it might be helpful for the broker to
send the Cluster ID as determined from its meta.properties file in its
registration request.  Does it even make sense for the broker to
successfully register and enter the Fenced state if it has the wrong
Cluster ID?  The nextMetadatOffset value that the broker communicates
in its registration request only has meaning within the correct
cluster, so it feels to me that the Controller should have some way to
perform this sanity check.  There is currently (pre-KIP 500) a check
in the broker to make sure its configured cluster ID matches the one
stored in ZooKeeper, and we will have to perform this validation
somewhere in the KIP-500 world.  If the Controller doesn't do it
within the registration request then the broker will have to make a
metadata request to the Controller, retrieve the Cluster ID, and
perform the check itself.  It feels to me that it might be better for
the Controller to just do it, and then the broker doesn't have to
worry about it anymore once it successfully registers.

I also have a question about the broker.id value and meta.properties.
The KIP now says "In version 0 of meta.properties, there is a
broker.id field.  Version 1 does not have this field.  It is no longer
needed because we no longer support dynamic broker id assignment."
But then there is an example version 1 meta.properties file that shows
the broker.id value.  I actually wonder if maybe the broker.id value
would be good to keep in the version 1 meta.properties file because it
currently (pre-KIP 500, version 0) acts as a sanity check to make sure
the broker is using the correct log directory.  Similarly with the
controller.id value on controllers -- it would allow the same type of
sanity check for quorum controllers.

Thanks again for your diligent leadership on this KIP.

Ron

On Mon, Nov 30, 2020 at 7:41 PM Colin McCabe <cmccabe@apache.org> wrote:
>
> On Fri, Oct 23, 2020, at 16:10, Jun Rao wrote:
> > Hi, Colin,
> >
> > Thanks for the reply. A few more comments.
>
> Hi Jun,
>
> Thanks again for the reply.  Sorry for the long hiatus.  I was on vacation for a while.
>
> >
> > 55. There is still text that favors new broker registration. "When a broker
> > first starts up, when it is in the INITIAL state, it will always "win"
> > broker ID conflicts.  However, once it is granted a lease, it transitions
> > out of the INITIAL state.  Thereafter, it may lose subsequent conflicts if
> > its broker epoch is stale.  (See KIP-380 for some background on broker
> > epoch.)  The reason for favoring new processes is to accommodate the common
> > case where a process is killed with kill -9 and then restarted.  We want it
> > to be able to reclaim its old ID quickly in this case."
> >
>
> Thanks for the reminder.  I have clarified the language here.  Hopefully now it is clear
that we don't allow quick re-use of broker IDs.
>
> > 80.1 Sounds good. Could you document that listeners is a required config
> > now? It would also be useful to annotate other required configs. For
> > example, controller.connect should be required.
> >
>
> I added a note specifying that these are required.
>
> > 80.2 Could you list all deprecated existing configs? Another one is
> > control.plane.listener.name since the controller no longer sends
> > LeaderAndIsr, UpdateMetadata and StopReplica requests.
> >
>
> I added a section specifying some deprecated configs.
>
> > 83.1 It seems that the broker can transition from FENCED to RUNNING without
> > registering for a new broker epoch. I am not sure how this works. Once the
> > controller fences a broker, there is no need for the controller to keep the
> > boker epoch around. So, if the fenced broker's heartbeat request with the
> > existing broker epoch will be rejected, leading the broker back to the
> > FENCED state again.
> >
>
> The broker epoch refers to the broker registration.  So we DO keep the broker epoch around
even while the broker is fenced.
>
> The broker epoch changes only when there is a new broker registration.  Fencing or unfencing
the broker doesn't change the broker epoch.
>
> > 83.5 Good point on KIP-590. Then should we expose the controller for
> > debugging purposes? If not, we should deprecate the controllerID field in
> > MetadataResponse?
> >
>
> I think it's OK to expose it for now, with the proviso that it won't be reachable by
clients.
>
> > 90. We rejected the shared ID with just one reason "This is not a good idea
> > because NetworkClient assumes a single ID space.  So if there is both a
> > controller 1 and a broker 1, we don't have a way of picking the "right"
> > one." This doesn't seem to be a strong reason. For example, we could
> > address the NetworkClient issue with the node type as you pointed out or
> > using the negative value of a broker ID as the controller ID.
> >
>
> It would require a lot of code changes to support multiple types of node IDs.  It's not
clear to me that the end result would be better -- I tend to think it would be worse, since
it would be more complex.  In a similar vein, using negative numbers seems dangerous, since
we use negatives or -1 as "special values" in many places.  For example, -1 often represents
"no such node."
>
> One important thing to keep in mind is that we want to be able to transition from a broker
and a controller being co-located to them no longer being co-located.  This is much easier
to do when they have separate IDs.
>
> > 100. In KIP-589
> > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-589+Add+API+to+update+Replica+state+in+Controller>,
> > the broker reports all offline replicas due to a disk failure to the
> > controller. It seems this information needs to be persisted to the
> > metadata
> > log. Do we have a corresponding record for that?
> >
>
> Hmm, I have to look into this a little bit more.  We may need a new record type.
>
> > 101. Currently, StopReplica request has 2 modes, without deletion and with
> > deletion. The former is used for controlled shutdown and handling disk
> > failure, and causes the follower to stop. The latter is for topic deletion
> > and partition reassignment, and causes the replica to be deleted. Since we
> > are deprecating StopReplica, could we document what triggers the stopping
> > of a follower and the deleting of a replica now?
> >
>
> RemoveTopic triggers deletion.  In general the functionality of StopReplica is subsumed
by the metadata records.
>
> > 102. Should we include the metadata topic in the MetadataResponse? If so,
> > when it will be included and what will the metadata response look like?
> >
>
> No, it won't be included in the metadata response sent back from the brokers.
>
> > 103. "The active controller assigns the broker a new broker epoch, based on
> > the latest committed offset in the log." This seems inaccurate since the
> > latest committed offset doesn't always advance on every log append.
> >
>
> Given that the new broker epoch won't be visible until the commit has happened, I have
changed this to "the next available offset in the log"
>
> > 104. REGISTERING(1) : It says "Otherwise, the broker moves into the FENCED
> > state.". It seems this should be RUNNING?
> >
> > 105. RUNNING: Should we require the broker to catch up to the metadata log
> > to get into this state?
>
> For 104 and 105, these sections have been reworked.
>
> best,
> Colin
>
> >
> > Thanks,
> >
> > Jun
> >
> >
> >
> > On Fri, Oct 23, 2020 at 1:20 PM Colin McCabe <cmccabe@apache.org> wrote:
> >
> > > On Wed, Oct 21, 2020, at 05:51, Tom Bentley wrote:
> > > > Hi Colin,
> > > >
> > > > On Mon, Oct 19, 2020, at 08:59, Ron Dagostino wrote:
> > > > > > Hi Colin.  Thanks for the hard work on this KIP.
> > > > > >
> > > > > > I have some questions about what happens to a broker when it
becomes
> > > > > > fenced (e.g. because it can't send a heartbeat request to keep
its
> > > > > > lease).  The KIP says "When a broker is fenced, it cannot process
any
> > > > > > client requests.  This prevents brokers which are not receiving
> > > > > > metadata updates or that are not receiving and processing them
fast
> > > > > > enough from causing issues to clients." And in the description
of the
> > > > > > FENCED(4) state it likewise says "While in this state, the broker
> > > does
> > > > > > not respond to client requests."  It makes sense that a fenced
broker
> > > > > > should not accept producer requests -- I assume any such requests
> > > > > > would result in NotLeaderOrFollowerException.  But what about
KIP-392
> > > > > > (fetch from follower) consumer requests?  It is conceivable
that
> > > these
> > > > > > could continue.  Related to that, would a fenced broker continue
to
> > > > > > fetch data for partitions where it thinks it is a follower?
 Even if
> > > > > > it rejects consumer requests it might still continue to fetch
as a
> > > > > > follower.  Might it be helpful to clarify both decisions here?
> > > > >
> > > > > Hi Ron,
> > > > >
> > > > > Good question.  I think a fenced broker should continue to fetch
on
> > > > > partitions it was already fetching before it was fenced, unless it
> > > hits a
> > > > > problem.  At that point it won't be able to continue, since it doesn't
> > > have
> > > > > the new metadata.  For example, it won't know about leadership changes
> > > in
> > > > > the partitions it's fetching.  The rationale for continuing to fetch
> > > is to
> > > > > try to avoid disruptions as much as possible.
> > > > >
> > > > > I don't think fenced brokers should accept client requests.  The
issue
> > > is
> > > > > that the fenced broker may or may not have any data it is supposed
to
> > > > > have.  It may or may not have applied any configuration changes,
etc.
> > > that
> > > > > it is supposed to have applied.  So it could get pretty confusing,
and
> > > also
> > > > > potentially waste the client's time.
> > > > >
> > > > >
> > > > When fenced, how would the broker reply to a client which did make a
> > > > request?
> > > >
> > >
> > > Hi Tom,
> > >
> > > The broker will respond with a retryable error in that case.  Once the
> > > client has re-fetched its metadata, it will no longer see the fenced broker
> > > as part of the cluster.  I added a note to the KIP.
> > >
> > > best,
> > > Colin
> > >
> > > >
> > > > Thanks,
> > > >
> > > > Tom
> > > >
> > >
> >

Mime
View raw message