Thanks for your feedback, David. Please find my answers below.

> When an unclean election is performed, I'm assuming we will ignore the two
new invariants proposed in the KIP? If so, maybe we should clarify that.

I am not sure about this. What's the point of electing a broker which
is fenced or shutting down in this case? At the moment, I think that
we already prevent a fenced broker from being uncleanly elected if not
mistaken. The KIP just extends this to include shutting down brokers
as well.

> I understand the use case of persisting the in-controller-shutdown state
for controller failover, but are we planning on using this state on the
leader side for ISR eligibility? Or will we just rely on the controller to
check this state.

Yes. The KIP explains that the leader will also enforce this based on
the information it gets via the metadata log. The idea is to prevent
unnecessary AlterPartition requests to the controller.

> I think we should bump the IBP/MetadataVersion since we are adding a new
field to a metadata record. Otherwise we might have trouble downgrading the
software if this field has been serialized.

Right. There is a note about this in the Compatibility section.

Best,
David

On Thu, Jun 2, 2022 at 6:04 PM David Arthur <mum...@gmail.com> wrote:
>
> David, thanks for the KIP!
>
> When an unclean election is performed, I'm assuming we will ignore the two
> new invariants proposed in the KIP? If so, maybe we should clarify that.
>
> I understand the use case of persisting the in-controller-shutdown state
> for controller failover, but are we planning on using this state on the
> leader side for ISR eligibility? Or will we just rely on the controller to
> check this state.
>
> I think we should bump the IBP/MetadataVersion since we are adding a new
> field to a metadata record. Otherwise we might have trouble downgrading the
> software if this field has been serialized.
>
> Cheers,
> David
>
> On Thu, Jun 2, 2022 at 11:04 AM David Jacot <dja...@confluent.io.invalid>
> wrote:
>
> > Hi José,
> >
> > At the moment, the KIP stipulates that the broker remains in
> > InControlledShutdown state until it is re-registered with a new
> > incarnation id. This implies that a broker can be both fenced and in
> > controlled shutdown state. We could make them mutually exclusive but I
> > think that there is value in the current proposal because we are able
> > to differentiate if a broker was fenced due to the controlled shutdown
> > or not.
> >
> > Best,
> > David
> >
> > On Wed, Jun 1, 2022 at 7:32 PM José Armando García Sancio
> > <jsan...@confluent.io.invalid> wrote:
> > >
> > > Thanks for the updates to the KIP.
> > >
> > > I like enumerating invariants. Is it safe to say that if
> > > `InControlledShutdown` is true then `Fenced` must be false.
> >
>
>
> --
> David Arthur

Reply via email to