Hi, David,
Thanks for the reply. No more questions from me.
Jun
On Tue, Nov 30, 2021 at 1:52 PM David Arthur
<david.art...@confluent.io.invalid> wrote:
> Thanks Jun,
>
> 30: I clarified the description of the "upgrade" command to:
>
> The FEATURE and VERSION arguments can be repeated to indicate an upgrade of
> > multiple features in one request. Alternatively, the RELEASE flag can be
> > given to upgrade to the default versions of the specified release. These
> > two options, FEATURE and RELEASE, are mutually exclusive. If neither is
> > given, this command will do nothing.
>
>
> Basically, you must provide either "kafka-features.sh upgrade --release
> 3.2" or something like "kafka-features.sh upgrade --feature X --version 10"
>
> -David
>
> On Tue, Nov 30, 2021 at 2:51 PM Jun Rao <j...@confluent.io.invalid> wrote:
>
> > Hi, David,
> >
> > Thanks for the reply. Just one more minor comment.
> >
> > 30. ./kafka-features.sh upgrade: It seems that the release param is
> > optional. Could you describe the semantic when release is not specified?
> >
> > Jun
> >
> > On Mon, Nov 29, 2021 at 5:06 PM David Arthur
> > <david.art...@confluent.io.invalid> wrote:
> >
> > > Jun, I updated the KIP with the "disable" CLI.
> > >
> > > For 16, I think you're asking how we can safely introduce the
> > > initial version of other feature flags in the future. This will
> probably
> > > depend on the nature of the feature that the new flag is gating. We can
> > > probably take a similar approach and say version 1 is backwards
> > compatible
> > > to some point (possibly an IBP or metadata.version?).
> > >
> > > -David
> > >
> > > On Fri, Nov 19, 2021 at 3:00 PM Jun Rao <j...@confluent.io.invalid>
> > wrote:
> > >
> > > > Hi, David,
> > > >
> > > > Thanks for the reply. The new CLI sounds reasonable to me.
> > > >
> > > > 16.
> > > > For case C, choosing the latest version sounds good to me.
> > > > For case B, for metadata.version, we pick version 1 since it just
> > happens
> > > > that for metadata.version version 1 is backward compatible. How do we
> > > make
> > > > this more general for other features?
> > > >
> > > > 21. Do you still plan to change "delete" to "disable" in the CLI?
> > > >
> > > > Thanks,
> > > >
> > > > Jun
> > > >
> > > >
> > > >
> > > > On Thu, Nov 18, 2021 at 11:50 AM David Arthur
> > > > <david.art...@confluent.io.invalid> wrote:
> > > >
> > > > > Jun,
> > > > >
> > > > > The KIP has some changes to the CLI for KIP-584. With Jason's
> > > suggestion
> > > > > incorporated, these three commands would look like:
> > > > >
> > > > > 1) kafka-features.sh upgrade --release latest
> > > > > upgrades all known features to their defaults in the latest release
> > > > >
> > > > > 2) kafka-features.sh downgrade --release 3.x
> > > > > downgrade all known features to the default versions from 3.x
> > > > >
> > > > > 3) kafka-features.sh describe --release latest
> > > > > Describe the known features of the latest release
> > > > >
> > > > > The --upgrade-all and --downgrade-all are replaced by specific each
> > > > > feature+version or giving the --release argument. One problem with
> > > > > --downgrade-all is it's not clear what the target versions should
> be:
> > > the
> > > > > previous version before the last upgrade -- or the lowest supported
> > > > > version. Since downgrades will be less common, I think it's better
> to
> > > > make
> > > > > the operator be more explicit about the desired downgrade version
> > > (either
> > > > > through [--feature X --version Y] or [--release 3.1]). Does that
> seem
> > > > > reasonable?
> > > > >
> > > > > 16. For case C, I think we will want to always use the latest
> > > > > metadata.version for new clusters (like we do for IBP). We can
> always
> > > > > change what we mean by "default" down the road. Also, this will
> > likely
> > > > > become a bit of information we include in release and upgrade notes
> > > with
> > > > > each release.
> > > > >
> > > > > -David
> > > > >
> > > > > On Thu, Nov 18, 2021 at 1:14 PM Jun Rao <j...@confluent.io.invalid>
> > > > wrote:
> > > > >
> > > > > > Hi, Jason, David,
> > > > > >
> > > > > > Just to clarify on the interaction with the end user, the design
> in
> > > > > KIP-584
> > > > > > allows one to do the following.
> > > > > > (1) kafka-features.sh --upgrade-all --bootstrap-server
> > > > > > kafka-broker0.prn1:9071 to upgrade all features to the latest
> > version
> > > > > known
> > > > > > by the tool. The user doesn't need to know a specific feature
> > > version.
> > > > > > (2) kafka-features.sh --downgrade-all --bootstrap-server
> > > > > > kafka-broker0.prn1:9071 to downgrade all features to the latest
> > > version
> > > > > > known by the tool. The user doesn't need to know a specific
> feature
> > > > > > version.
> > > > > > (3) kafka-features.sh --describe --bootstrap-server
> > > > > > kafka-broker0.prn1:9071 to find out the supported version for
> each
> > > > > feature.
> > > > > > This allows the user to upgrade each feature individually.
> > > > > >
> > > > > > Most users will be doing (1) and occasionally (2), and won't need
> > to
> > > > know
> > > > > > the exact version of each feature.
> > > > > >
> > > > > > 16. For case C, what's the default version? Is it always the
> > latest?
> > > > > >
> > > > > > Thanks,
> > > > > >
> > > > > > Jun
> > > > > >
> > > > > >
> > > > > > On Thu, Nov 18, 2021 at 8:15 AM David Arthur
> > > > > > <david.art...@confluent.io.invalid> wrote:
> > > > > >
> > > > > > > Colin, thanks for the detailed response. I understand what
> you're
> > > > > saying
> > > > > > > and I agree with your rationale.
> > > > > > >
> > > > > > > It seems like we could just initialize their cluster.metadata
> to
> > 1
> > > > when
> > > > > > the
> > > > > > > > software is upgraded to 3.2.
> > > > > > > >
> > > > > > >
> > > > > > > Concretely, this means the controller would see that there is
> no
> > > > > > > FeatureLevelRecord in the log, and so it would bootstrap one.
> > > > Normally
> > > > > > for
> > > > > > > cluster initialization, this would be read from
> meta.properties,
> > > but
> > > > in
> > > > > > the
> > > > > > > case of preview clusters we would need to special case it to
> > > > initialize
> > > > > > the
> > > > > > > version to 1.
> > > > > > >
> > > > > > > Once the new FeatureLevelRecord has been committed, any nodes
> > > > (brokers
> > > > > or
> > > > > > > controllers) that are running the latest software will react to
> > the
> > > > new
> > > > > > > metadata.version. This means we will need to make this initial
> > > > version
> > > > > > of 1
> > > > > > > be backwards compatible to what we have in 3.0 and 3.1 (since
> > some
> > > > > > brokers
> > > > > > > will be on the new software and some on older/preview versions)
> > > > > > >
> > > > > > > I agree that auto-upgrading preview users from IBP 3.0 to
> > > > > > metadata.version
> > > > > > > 1 (equivalent to IBP 3.2) is probably fine.
> > > > > > >
> > > > > > > Back to Jun's case B:
> > > > > > >
> > > > > > > b. We upgrade from an old version where no metadata.version has
> > > been
> > > > > > > > finalized. In this case, it makes sense to leave
> > metadata.version
> > > > > > > disabled
> > > > > > > > since we don't know if all brokers have been upgraded.
> > > > > > >
> > > > > > >
> > > > > > > Instead of leaving it uninitialized, we would initialize it to
> 1
> > > > which
> > > > > > > would be backwards compatible to 3.0 and 3.1. This would
> > eliminate
> > > > the
> > > > > > need
> > > > > > > to check a "final IBP" or deal with 3.2+ clusters without a
> > > > > > > metadata.version set. The downgrade path for 3.2 back to a
> > preview
> > > > > > release
> > > > > > > should be fine since we are saying that metadata.version 1 is
> > > > > compatible
> > > > > > > with those releases. The FeatureLevelRecord would exist, but it
> > > would
> > > > > be
> > > > > > > ignored (we might need to make sure this actually works in
> 3.0).
> > > > > > >
> > > > > > > For clarity, here are the three workflows of Jun's three cases:
> > > > > > >
> > > > > > > A (KRaft 3.2+ to KRaft 3.2+):
> > > > > > > * User initializes cluster with an explicit metadata.version X,
> > or
> > > > the
> > > > > > tool
> > > > > > > selects the default
> > > > > > > * After upgrade, cluster stays at version X until operator
> > upgrades
> > > > to
> > > > > Y
> > > > > > >
> > > > > > > B (KRaft Preview to KRaft 3.2+):
> > > > > > > * User initializes cluster without metadata.version
> > > > > > > * After controller leader is upgraded, initializes
> > metadata.version
> > > > to
> > > > > 1
> > > > > > > * After the cluster is upgraded, an operator may further
> upgrade
> > to
> > > > > > version
> > > > > > > Y
> > > > > > >
> > > > > > > C (New KRaft 3.2+):
> > > > > > > * User initializes cluster with an explicit metadata.version X,
> > or
> > > > the
> > > > > > tool
> > > > > > > selects the default
> > > > > > >
> > > > > > >
> > > > > > > This has been mentioned in this thread, but we should
> explicitly
> > > call
> > > > > out
> > > > > > > that the absence of metadata.version in meta.properties will be
> > > used
> > > > > > > to identify that we are in case B. After 3.2, we will require
> > that
> > > > > > > metadata.version is present in meta.properties for new
> clusters.
> > If
> > > > > users
> > > > > > > omit the --metadata-version flag from kafka-storage.sh, we
> should
> > > > fill
> > > > > > in a
> > > > > > > default.
> > > > > > >
> > > > > > > So how about the following changes/clarifications to the KIP:
> > > > > > >
> > > > > > > * Starting with 3.2, metadata.version is required in KRaft, IBP
> > is
> > > > > > ignored
> > > > > > > * If meta.properties does not include metadata.version, it
> > > indicates
> > > > > this
> > > > > > > cluster was initialized with a preview release
> > > > > > > * If upgrading from a preview release to 3.2+, the controller
> > will
> > > > > > > initialize metadata.version=1
> > > > > > > * metadata.version=1 is backwards compatible with preview
> > releases
> > > > > > >
> > > > > > > -David
> > > > > > >
> > > > > > >
> > > > > > > On Thu, Nov 18, 2021 at 10:02 AM David Arthur <
> > > > > david.art...@confluent.io
> > > > > > >
> > > > > > > wrote:
> > > > > > >
> > > > > > > > Jason,
> > > > > > > >
> > > > > > > > 1/2. You've got it right. The intention is that
> > metadata.version
> > > > will
> > > > > > > gate
> > > > > > > > both RPCs (like IBP does) as well as metadata records. So,
> > when a
> > > > > > broker
> > > > > > > > sees that metadata.version changed, it may start advertising
> > new
> > > > RPCs
> > > > > > and
> > > > > > > > it will need to refresh the ApiVersions it has for other
> > > brokers. I
> > > > > can
> > > > > > > try
> > > > > > > > to make this more explicit in the KIP.
> > > > > > > >
> > > > > > > > 3. Yes, that's the intention of the --dry-run flag. The
> current
> > > > > > > > implementation in kafka-features.sh does a dry run on the
> > client
> > > > > side,
> > > > > > > but
> > > > > > > > this KIP pushes the validation down to the controller. This
> > will
> > > > > allow
> > > > > > us
> > > > > > > > to have the context needed to do proper validation
> > > > > > > >
> > > > > > > > Re: version number complexity -- yes this has come up in
> > offline
> > > > > > > > discussions. With just one feature flag to manage, it's not
> so
> > > bad,
> > > > > but
> > > > > > > > explicitly managing even a few would be a burden. I like your
> > > > > > suggestion
> > > > > > > of
> > > > > > > > the "--release" flag. That could act as a sort of manifest of
> > > > > versions
> > > > > > > > (i.e., the defaults from that release). We could also support
> > > > > something
> > > > > > > > like "kafka-features upgrade --release latest" to bring
> > > everything
> > > > to
> > > > > > the
> > > > > > > > highest supported version.
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > On Wed, Nov 17, 2021 at 9:36 PM Jason Gustafson
> > > > > > > <ja...@confluent.io.invalid>
> > > > > > > > wrote:
> > > > > > > >
> > > > > > > >> Hi Colin/David,
> > > > > > > >>
> > > > > > > >> > Like David said, basically it boils down to creating a
> > feature
> > > > > flag
> > > > > > > for
> > > > > > > >> the new proposed __consumer_offsets version, or using a new
> > > > > > > >> IBP/metadata.version for it. Both approaches have pros and
> > cons.
> > > > > Using
> > > > > > > an
> > > > > > > >> IBP/metadata.version bump reduces the size of the testing
> > > matrix.
> > > > > But
> > > > > > > >> using
> > > > > > > >> a feature flag allows people to avoid any bugs or pain
> > > associated
> > > > > with
> > > > > > > the
> > > > > > > >> change if they don't care about enabling it. This is
> basically
> > > the
> > > > > > > classic
> > > > > > > >> "should I use a feature flag or not?" discussion and we need
> > to
> > > > have
> > > > > > it
> > > > > > > on
> > > > > > > >> a case-by-case basis.
> > > > > > > >>
> > > > > > > >> I think most users are not going to care to manage versions
> > for
> > > a
> > > > > > bunch
> > > > > > > of
> > > > > > > >> different features. The IBP today has many shortcomings, but
> > at
> > > > > least
> > > > > > > it's
> > > > > > > >> tied to a version that users understand (i.e. the release
> > > > version).
> > > > > > How
> > > > > > > >> would users know after upgrading to Kafka 3.1, for example,
> > that
> > > > > they
> > > > > > > need
> > > > > > > >> to upgrade the metadata.version to 3 and offsets.version
> to 4
> > > (or
> > > > > > > >> whatever)? It's a lot of overhead trying to understand all
> of
> > > the
> > > > > > > >> potential
> > > > > > > >> features and what each upgrade actually means to them. I am
> > > > > wondering
> > > > > > if
> > > > > > > >> we
> > > > > > > >> could give them something more convenient which is tied to
> the
> > > > > release
> > > > > > > >> version. For example, maybe we could use a command like
> > > > > > `kafka-features
> > > > > > > >> upgrade --release 3.1`, which the broker would then
> translate
> > to
> > > > an
> > > > > > > >> upgrade
> > > > > > > >> to the latest versions of the individual features at the
> time
> > of
> > > > the
> > > > > > 3.1
> > > > > > > >> release. Basically it's just a static map from release
> version
> > > to
> > > > > > > feature
> > > > > > > >> versions to make the upgrade process more convenient.
> > > > > > > >>
> > > > > > > >> Thanks,
> > > > > > > >> Jason
> > > > > > > >>
> > > > > > > >>
> > > > > > > >>
> > > > > > > >>
> > > > > > > >> On Wed, Nov 17, 2021 at 6:20 PM Jason Gustafson <
> > > > ja...@confluent.io
> > > > > >
> > > > > > > >> wrote:
> > > > > > > >>
> > > > > > > >> > A few additional questions:
> > > > > > > >> >
> > > > > > > >> > 1. Currently the IBP tells us what version of individual
> > > > > > inter-broker
> > > > > > > >> RPCs
> > > > > > > >> > will be used. I think the plan in this KIP is to use
> > > ApiVersions
> > > > > > > request
> > > > > > > >> > instead to find the highest compatible version (just like
> > > > > clients).
> > > > > > > Do I
> > > > > > > >> > have that right?
> > > > > > > >> >
> > > > > > > >> > 2. The following wasn't very clear to me:
> > > > > > > >> >
> > > > > > > >> > > Brokers will be able to observe changes to
> > metadata.version
> > > by
> > > > > > > >> observing
> > > > > > > >> > the metadata log, and could then submit a new
> > > ApiVersionsRequest
> > > > > to
> > > > > > > the
> > > > > > > >> > other Kafka nodes.
> > > > > > > >> >
> > > > > > > >> > Is the purpose of submitting new ApiVersions requests to
> > > update
> > > > > the
> > > > > > > >> > features or the RPC versions? Does metadata.version also
> > > > influence
> > > > > > the
> > > > > > > >> > versions that a broker advertises? It would help to have
> > more
> > > > > detail
> > > > > > > >> about
> > > > > > > >> > this.
> > > > > > > >> >
> > > > > > > >> > 3. I imagine users will want to know before performing an
> > > > upgrade
> > > > > > > >> whether
> > > > > > > >> > downgrading will be safe. Would the --dry-run flag tell
> them
> > > > this?
> > > > > > > >> >
> > > > > > > >> > Thanks,
> > > > > > > >> > Jason
> > > > > > > >> >
> > > > > > > >> >
> > > > > > > >> >
> > > > > > > >> >
> > > > > > > >> >
> > > > > > > >> > On Wed, Nov 17, 2021 at 3:55 PM Colin McCabe <
> > > > cmcc...@apache.org>
> > > > > > > >> wrote:
> > > > > > > >> >
> > > > > > > >> >> On Wed, Nov 17, 2021, at 11:28, Jason Gustafson wrote:
> > > > > > > >> >> > Hi David,
> > > > > > > >> >> >
> > > > > > > >> >> > Forgive me if this ground has been covered already.
> > Today,
> > > we
> > > > > > have
> > > > > > > a
> > > > > > > >> few
> > > > > > > >> >> > other things that we have latched onto the IBP, such as
> > > > > upgrades
> > > > > > to
> > > > > > > >> the
> > > > > > > >> >> > format of records in __consumer_offsets. I've been
> > assuming
> > > > > that
> > > > > > > >> >> > metadata.version is not covering this. Is that right or
> > is
> > > > > there
> > > > > > > some
> > > > > > > >> >> other
> > > > > > > >> >> > plan to take care of cases like this?
> > > > > > > >> >> >
> > > > > > > >> >>
> > > > > > > >> >> I think metadata.version could cover changes to things
> like
> > > > > > > >> >> __consumer_offsets, if people want it to. Or to put it
> > > another
> > > > > way,
> > > > > > > >> that is
> > > > > > > >> >> out of scope for this KIP.
> > > > > > > >> >>
> > > > > > > >> >> Like David said, basically it boils down to creating a
> > > feature
> > > > > flag
> > > > > > > for
> > > > > > > >> >> the new proposed __consumer_offsets version, or using a
> new
> > > > > > > >> >> IBP/metadata.version for it. Both approaches have pros
> and
> > > > cons.
> > > > > > > Using
> > > > > > > >> an
> > > > > > > >> >> IBP/metadata.version bump reduces the size of the testing
> > > > matrix.
> > > > > > But
> > > > > > > >> using
> > > > > > > >> >> a feature flag allows people to avoid any bugs or pain
> > > > associated
> > > > > > > with
> > > > > > > >> the
> > > > > > > >> >> change if they don't care about enabling it. This is
> > > basically
> > > > > the
> > > > > > > >> classic
> > > > > > > >> >> "should I use a feature flag or not?" discussion and we
> > need
> > > to
> > > > > > have
> > > > > > > >> it on
> > > > > > > >> >> a case-by-case basis.
> > > > > > > >> >>
> > > > > > > >> >> I think it's worth calling out that having a 1:1 mapping
> > > > between
> > > > > > IBP
> > > > > > > >> >> versions and metadata.versions will result in some
> > > > > > metadata.versions
> > > > > > > >> that
> > > > > > > >> >> "don't do anything" (aka they do the same thing as the
> > > previous
> > > > > > > >> >> metadata.version). For example, if we change
> > > StopReplicaRequest
> > > > > > > again,
> > > > > > > >> that
> > > > > > > >> >> will not affect KRaft mode, but probably would require an
> > IBP
> > > > > bump
> > > > > > > and
> > > > > > > >> >> hence a metadata.version bump. I think that's OK. It's
> not
> > > that
> > > > > > > >> different
> > > > > > > >> >> from updating your IBP and getting support for ZStandard,
> > > when
> > > > > your
> > > > > > > >> >> deployment doesn't use ZStandard compression.
> > > > > > > >> >>
> > > > > > > >> >> best,
> > > > > > > >> >> Colin
> > > > > > > >> >>
> > > > > > > >> >> > Thanks,
> > > > > > > >> >> > Jason
> > > > > > > >> >> >
> > > > > > > >> >> >
> > > > > > > >> >> >
> > > > > > > >> >> > On Wed, Nov 17, 2021 at 10:17 AM Jun Rao
> > > > > > <j...@confluent.io.invalid
> > > > > > > >
> > > > > > > >> >> wrote:
> > > > > > > >> >> >
> > > > > > > >> >> >> Hi, Colin,
> > > > > > > >> >> >>
> > > > > > > >> >> >> Thanks for the reply.
> > > > > > > >> >> >>
> > > > > > > >> >> >> For case b, I am not sure that I understand your
> > > suggestion.
> > > > > > Does
> > > > > > > >> "each
> > > > > > > >> >> >> subsequent level for metadata.version corresponds to
> an
> > > IBP
> > > > > > > version"
> > > > > > > >> >> mean
> > > > > > > >> >> >> that we need to keep IBP forever? Could you describe
> the
> > > > > upgrade
> > > > > > > >> >> process in
> > > > > > > >> >> >> this case?
> > > > > > > >> >> >>
> > > > > > > >> >> >> Jun
> > > > > > > >> >> >>
> > > > > > > >> >> >> On Tue, Nov 16, 2021 at 3:45 PM Colin McCabe <
> > > > > > cmcc...@apache.org>
> > > > > > > >> >> wrote:
> > > > > > > >> >> >>
> > > > > > > >> >> >> > On Tue, Nov 16, 2021, at 15:13, Jun Rao wrote:
> > > > > > > >> >> >> > > Hi, David, Colin,
> > > > > > > >> >> >> > >
> > > > > > > >> >> >> > > Thanks for the reply.
> > > > > > > >> >> >> > >
> > > > > > > >> >> >> > > 16. Discussed with David offline a bit. We have 3
> > > cases.
> > > > > > > >> >> >> > > a. We upgrade from an old version where the
> > > > > metadata.version
> > > > > > > has
> > > > > > > >> >> >> already
> > > > > > > >> >> >> > > been finalized. In this case it makes sense to
> stay
> > > with
> > > > > > that
> > > > > > > >> >> feature
> > > > > > > >> >> >> > > version after the upgrade.
> > > > > > > >> >> >> >
> > > > > > > >> >> >> > +1
> > > > > > > >> >> >> >
> > > > > > > >> >> >> > > b. We upgrade from an old version where no
> > > > > metadata.version
> > > > > > > has
> > > > > > > >> >> been
> > > > > > > >> >> >> > > finalized. In this case, it makes sense to leave
> > > > > > > >> metadata.version
> > > > > > > >> >> >> > disabled
> > > > > > > >> >> >> > > since we don't know if all brokers have been
> > upgraded.
> > > > > > > >> >> >> >
> > > > > > > >> >> >> > This is the scenario I was hoping to avoid by saying
> > > that
> > > > > ALL
> > > > > > > >> KRaft
> > > > > > > >> >> >> > clusters have metadata.version of at least 1, and
> each
> > > > > > > subsequent
> > > > > > > >> >> level
> > > > > > > >> >> >> for
> > > > > > > >> >> >> > metadata.version corresponds to an IBP version. The
> > > > existing
> > > > > > > KRaft
> > > > > > > >> >> >> clusters
> > > > > > > >> >> >> > in 3.0 and earlier are preview (not for production)
> > so I
> > > > > think
> > > > > > > >> this
> > > > > > > >> >> >> change
> > > > > > > >> >> >> > is OK for 3.x (given that it affects only KRaft).
> Then
> > > IBP
> > > > > is
> > > > > > > >> >> irrelevant
> > > > > > > >> >> >> > for KRaft clusters (the config is ignored, possibly
> > > with a
> > > > > > WARN
> > > > > > > or
> > > > > > > >> >> ERROR
> > > > > > > >> >> >> > message generated if it is set).
> > > > > > > >> >> >> >
> > > > > > > >> >> >> > > c. We are starting from a brand new cluster and of
> > > > course
> > > > > no
> > > > > > > >> >> >> > > metadata.version has been finalized. In this case,
> > the
> > > > KIP
> > > > > > > says
> > > > > > > >> it
> > > > > > > >> >> will
> > > > > > > >> >> >> > > pick the metadata.version in meta.properties. In
> the
> > > > > common
> > > > > > > >> case,
> > > > > > > >> >> >> people
> > > > > > > >> >> >> > > probably won't set the metadata.version in the
> > > > > > meta.properties
> > > > > > > >> file
> > > > > > > >> >> >> > > explicitly. So, it will be useful to put a default
> > > > > (stable)
> > > > > > > >> version
> > > > > > > >> >> >> there
> > > > > > > >> >> >> > > when the meta.properties.
> > > > > > > >> >> >> >
> > > > > > > >> >> >> > Hmm. I was assuming that clusters where the admin
> > didn't
> > > > > > specify
> > > > > > > >> any
> > > > > > > >> >> >> > metadata.version during formatting would get the
> > latest
> > > > > > > >> >> metadata.version.
> > > > > > > >> >> >> > Partly, because this is what we do for IBP today. It
> > > would
> > > > > be
> > > > > > > >> good to
> > > > > > > >> >> >> > clarify this...
> > > > > > > >> >> >> >
> > > > > > > >> >> >> > >
> > > > > > > >> >> >> > > Also, it would be useful to clarify that if a
> > > > > > > FeatureLevelRecord
> > > > > > > >> >> exists
> > > > > > > >> >> >> > for
> > > > > > > >> >> >> > > metadata.version, the metadata.version in
> > > > meta.properties
> > > > > > will
> > > > > > > >> be
> > > > > > > >> >> >> > ignored.
> > > > > > > >> >> >> > >
> > > > > > > >> >> >> >
> > > > > > > >> >> >> > Yeah, I agree.
> > > > > > > >> >> >> >
> > > > > > > >> >> >> > best,
> > > > > > > >> >> >> > Colin
> > > > > > > >> >> >> >
> > > > > > > >> >> >> > > Thanks,
> > > > > > > >> >> >> > >
> > > > > > > >> >> >> > > Jun
> > > > > > > >> >> >> > >
> > > > > > > >> >> >> > >
> > > > > > > >> >> >> > > On Tue, Nov 16, 2021 at 12:39 PM Colin McCabe <
> > > > > > > >> cmcc...@apache.org>
> > > > > > > >> >> >> > wrote:
> > > > > > > >> >> >> > >
> > > > > > > >> >> >> > >> On Fri, Nov 5, 2021, at 15:18, Jun Rao wrote:
> > > > > > > >> >> >> > >> > Hi, David,
> > > > > > > >> >> >> > >> >
> > > > > > > >> >> >> > >> > Thanks for the reply.
> > > > > > > >> >> >> > >> >
> > > > > > > >> >> >> > >> > 16. My first concern is that the KIP picks up
> > > > > > meta.version
> > > > > > > >> >> >> > inconsistently
> > > > > > > >> >> >> > >> > during the deployment. If a new cluster is
> > started,
> > > > we
> > > > > > pick
> > > > > > > >> up
> > > > > > > >> >> the
> > > > > > > >> >> >> > >> highest
> > > > > > > >> >> >> > >> > version. If we upgrade, we leave the feature
> > > version
> > > > > > > >> unchanged.
> > > > > > > >> >> >> > >>
> > > > > > > >> >> >> > >> Hi Jun,
> > > > > > > >> >> >> > >>
> > > > > > > >> >> >> > >> Thanks again for taking a look.
> > > > > > > >> >> >> > >>
> > > > > > > >> >> >> > >> The proposed behavior in KIP-778 is consistent
> with
> > > how
> > > > > it
> > > > > > > >> works
> > > > > > > >> >> >> today.
> > > > > > > >> >> >> > >> Upgrading the software is distinct from upgrading
> > the
> > > > > IBP.
> > > > > > > >> >> >> > >>
> > > > > > > >> >> >> > >> I think it is important to keep these two
> > operations
> > > > > > > >> ("upgrading
> > > > > > > >> >> >> > >> IBP/metadata version" and "upgrading software
> > > version")
> > > > > > > >> separate.
> > > > > > > >> >> If
> > > > > > > >> >> >> > they
> > > > > > > >> >> >> > >> are coupled it will create a situation where
> > software
> > > > > > > upgrades
> > > > > > > >> are
> > > > > > > >> >> >> > >> difficult and dangerous.
> > > > > > > >> >> >> > >>
> > > > > > > >> >> >> > >> Consider a situation where you find some bug in
> > your
> > > > > > current
> > > > > > > >> >> software,
> > > > > > > >> >> >> > and
> > > > > > > >> >> >> > >> you want to upgrade to new software that fixes
> the
> > > bug.
> > > > > If
> > > > > > > >> >> upgrades
> > > > > > > >> >> >> and
> > > > > > > >> >> >> > IBP
> > > > > > > >> >> >> > >> bumps are coupled, you can't do this without also
> > > > bumping
> > > > > > the
> > > > > > > >> IBP,
> > > > > > > >> >> >> > which is
> > > > > > > >> >> >> > >> usually considered a high-risk change. That means
> > > that
> > > > > > either
> > > > > > > >> you
> > > > > > > >> >> have
> > > > > > > >> >> >> > to
> > > > > > > >> >> >> > >> make a special build that includes only the fix
> > > > > > > (time-consuming
> > > > > > > >> >> and
> > > > > > > >> >> >> > >> error-prone), live with the bug for longer, or be
> > > very
> > > > > > > >> >> conservative
> > > > > > > >> >> >> > about
> > > > > > > >> >> >> > >> ever introducing new IBP/metadata versions. None
> of
> > > > those
> > > > > > are
> > > > > > > >> >> really
> > > > > > > >> >> >> > good
> > > > > > > >> >> >> > >> choices.
> > > > > > > >> >> >> > >>
> > > > > > > >> >> >> > >> > Intuitively, it seems that independent of how a
> > > > cluster
> > > > > > is
> > > > > > > >> >> deployed,
> > > > > > > >> >> >> > we
> > > > > > > >> >> >> > >> > should always pick the same feature version.
> > > > > > > >> >> >> > >>
> > > > > > > >> >> >> > >> I think it makes sense to draw a distinction
> > between
> > > > > > > upgrading
> > > > > > > >> an
> > > > > > > >> >> >> > existing
> > > > > > > >> >> >> > >> cluster and deploying a new one. What most people
> > > want
> > > > > out
> > > > > > of
> > > > > > > >> >> upgrades
> > > > > > > >> >> >> > is
> > > > > > > >> >> >> > >> that things should keep working, but with bug
> > fixes.
> > > If
> > > > > we
> > > > > > > >> change
> > > > > > > >> >> >> that,
> > > > > > > >> >> >> > it
> > > > > > > >> >> >> > >> just makes people more reluctant to upgrade
> (which
> > is
> > > > > > always
> > > > > > > a
> > > > > > > >> >> >> > problem...)
> > > > > > > >> >> >> > >>
> > > > > > > >> >> >> > >> > I think we need to think this through in this
> > KIP.
> > > My
> > > > > > > second
> > > > > > > >> >> concern
> > > > > > > >> >> >> > is
> > > > > > > >> >> >> > >> > that as a particular version matures, it's
> > > > inconvenient
> > > > > > > for a
> > > > > > > >> >> user
> > > > > > > >> >> >> to
> > > > > > > >> >> >> > >> manually
> > > > > > > >> >> >> > >> > upgrade every feature version. As long as we
> > have a
> > > > > path
> > > > > > to
> > > > > > > >> >> achieve
> > > > > > > >> >> >> > that
> > > > > > > >> >> >> > >> in
> > > > > > > >> >> >> > >> > the future, we don't need to address that in
> this
> > > > KIP.
> > > > > > > >> >> >> > >>
> > > > > > > >> >> >> > >> If people are managing a large number of Kafka
> > > > clusters,
> > > > > > they
> > > > > > > >> will
> > > > > > > >> >> >> want
> > > > > > > >> >> >> > to
> > > > > > > >> >> >> > >> do some sort of A/B testing with IBP/metadata
> > > versions.
> > > > > So
> > > > > > if
> > > > > > > >> you
> > > > > > > >> >> have
> > > > > > > >> >> >> > 1000
> > > > > > > >> >> >> > >> Kafka clusters, you roll out the new IBP version
> to
> > > 10
> > > > of
> > > > > > > them
> > > > > > > >> >> and see
> > > > > > > >> >> >> > how
> > > > > > > >> >> >> > >> it goes. If that goes well, you roll it out to
> > more,
> > > > etc.
> > > > > > > >> >> >> > >>
> > > > > > > >> >> >> > >> So, the automation needs to be at the cluster
> > > > management
> > > > > > > layer,
> > > > > > > >> >> not at
> > > > > > > >> >> >> > the
> > > > > > > >> >> >> > >> Kafka layer. Each Kafka cluster doesn't know how
> > well
> > > > > > things
> > > > > > > >> went
> > > > > > > >> >> in
> > > > > > > >> >> >> the
> > > > > > > >> >> >> > >> other 999 clusters. Automatically upgrading is a
> > bad
> > > > > thing
> > > > > > > for
> > > > > > > >> the
> > > > > > > >> >> >> same
> > > > > > > >> >> >> > >> reason Kafka automatically upgrading its own
> > software
> > > > > > version
> > > > > > > >> >> would
> > > > > > > >> >> >> be a
> > > > > > > >> >> >> > >> bad thing -- it could lead to a disruption to a
> > > > sensitive
> > > > > > > >> cluster
> > > > > > > >> >> at
> > > > > > > >> >> >> the
> > > > > > > >> >> >> > >> wrong time.
> > > > > > > >> >> >> > >>
> > > > > > > >> >> >> > >> For people who are just managing one or two Kafka
> > > > > clusters,
> > > > > > > >> >> >> > automatically
> > > > > > > >> >> >> > >> upgrading feature versions isn't a big burden and
> > can
> > > > be
> > > > > > done
> > > > > > > >> >> >> manually.
> > > > > > > >> >> >> > >> This is all consistent with how IBP works today.
> > > > > > > >> >> >> > >>
> > > > > > > >> >> >> > >> Also, we already have a command-line option to
> the
> > > > > feature
> > > > > > > tool
> > > > > > > >> >> which
> > > > > > > >> >> >> > >> upgrades all features to the latest available, if
> > > that
> > > > is
> > > > > > > what
> > > > > > > >> the
> > > > > > > >> >> >> > >> administrator desires. Perhaps we could add
> > > > documentation
> > > > > > > >> saying
> > > > > > > >> >> that
> > > > > > > >> >> >> > this
> > > > > > > >> >> >> > >> should be done as the last step of the upgrade.
> > > > > > > >> >> >> > >>
> > > > > > > >> >> >> > >> best,
> > > > > > > >> >> >> > >> Colin
> > > > > > > >> >> >> > >>
> > > > > > > >> >> >> > >> >
> > > > > > > >> >> >> > >> > 21. "./kafka-features.sh delete": Deleting a
> > > feature
> > > > > > seems
> > > > > > > a
> > > > > > > >> bit
> > > > > > > >> >> >> weird
> > > > > > > >> >> >> > >> > since the logic is always there. Would it be
> > better
> > > > to
> > > > > > use
> > > > > > > >> >> disable?
> > > > > > > >> >> >> > >> >
> > > > > > > >> >> >> > >> > Jun
> > > > > > > >> >> >> > >> >
> > > > > > > >> >> >> > >> > On Fri, Nov 5, 2021 at 8:11 AM David Arthur
> > > > > > > >> >> >> > >> > <david.art...@confluent.io.invalid> wrote:
> > > > > > > >> >> >> > >> >
> > > > > > > >> >> >> > >> >> Colin and Jun, thanks for the additional
> > comments!
> > > > > > > >> >> >> > >> >>
> > > > > > > >> >> >> > >> >> Colin:
> > > > > > > >> >> >> > >> >>
> > > > > > > >> >> >> > >> >> > We've been talking about having an automated
> > RPC
> > > > > > > >> >> compatibility
> > > > > > > >> >> >> > checker
> > > > > > > >> >> >> > >> >>
> > > > > > > >> >> >> > >> >> Do we have a way to mark fields in schemas as
> > > > > > deprecated?
> > > > > > > It
> > > > > > > >> >> can
> > > > > > > >> >> >> > stay in
> > > > > > > >> >> >> > >> >> the RPC, it just complicates the logic a bit.
> > > > > > > >> >> >> > >> >>
> > > > > > > >> >> >> > >> >> > It would be nice if the active controller
> > could
> > > > > > validate
> > > > > > > >> >> that a
> > > > > > > >> >> >> > >> majority
> > > > > > > >> >> >> > >> >> of the quorum could use the proposed
> > > > metadata.version.
> > > > > > The
> > > > > > > >> >> active
> > > > > > > >> >> >> > >> >> controller should have this information,
> right?
> > If
> > > > we
> > > > > > > don't
> > > > > > > >> >> have
> > > > > > > >> >> >> > recent
> > > > > > > >> >> >> > >> >> information from a quorum of voters, we
> > wouldn't
> > > be
> > > > > > > active.
> > > > > > > >> >> >> > >> >>
> > > > > > > >> >> >> > >> >> I believe we should have this information from
> > the
> > > > > > > >> >> >> > ApiVersionsResponse.
> > > > > > > >> >> >> > >> It
> > > > > > > >> >> >> > >> >> would be good to do this validation to avoid a
> > > > > situation
> > > > > > > >> where
> > > > > > > >> >> a
> > > > > > > >> >> >> > >> >> quorum leader can't be elected due to
> > > unprocessable
> > > > > > > records.
> > > > > > > >> >> >> > >> >>
> > > > > > > >> >> >> > >> >> > Do we need delete as a command separate from
> > > > > > downgrade?
> > > > > > > >> >> >> > >> >>
> > > > > > > >> >> >> > >> >> I think from an operator's perspective, it is
> > nice
> > > > to
> > > > > > > >> >> distinguish
> > > > > > > >> >> >> > >> between
> > > > > > > >> >> >> > >> >> changing a feature flag and unsetting it. It
> > might
> > > > be
> > > > > > > >> >> surprising to
> > > > > > > >> >> >> > an
> > > > > > > >> >> >> > >> >> operator to see the flag's version set to
> > nothing
> > > > when
> > > > > > > they
> > > > > > > >> >> >> requested
> > > > > > > >> >> >> > >> the
> > > > > > > >> >> >> > >> >> downgrade to version 0 (or less).
> > > > > > > >> >> >> > >> >>
> > > > > > > >> >> >> > >> >> > it seems like we should spell out that
> > > > > > metadata.version
> > > > > > > >> >> begins at
> > > > > > > >> >> >> > 1 in
> > > > > > > >> >> >> > >> >> KRaft clusters
> > > > > > > >> >> >> > >> >>
> > > > > > > >> >> >> > >> >> I added this text:
> > > > > > > >> >> >> > >> >>
> > > > > > > >> >> >> > >> >> Introduce an IBP version to indicate the
> lowest
> > > > > software
> > > > > > > >> >> version
> > > > > > > >> >> >> that
> > > > > > > >> >> >> > >> >> > supports *metadata.version*. Below this IBP,
> > the
> > > > > > > >> >> >> > *metadata.version* is
> > > > > > > >> >> >> > >> >> > undefined and will not be examined. At or
> > above
> > > > this
> > > > > > > IBP,
> > > > > > > >> the
> > > > > > > >> >> >> > >> >> > *metadata.version* must be *0* for ZooKeeper
> > > > > clusters
> > > > > > > and
> > > > > > > >> >> will be
> > > > > > > >> >> >> > >> >> > initialized as *1* for KRaft clusters.
> > > > > > > >> >> >> > >> >>
> > > > > > > >> >> >> > >> >>
> > > > > > > >> >> >> > >> >> > We probably also want an RPC implemented by
> > both
> > > > > > brokers
> > > > > > > >> and
> > > > > > > >> >> >> > >> controllers
> > > > > > > >> >> >> > >> >> that will reveal the min and max supported
> > > versions
> > > > > for
> > > > > > > each
> > > > > > > >> >> >> feature
> > > > > > > >> >> >> > >> level
> > > > > > > >> >> >> > >> >> supported by the server
> > > > > > > >> >> >> > >> >>
> > > > > > > >> >> >> > >> >> This is available in ApiVersionsResponse (we
> > > include
> > > > > the
> > > > > > > >> >> server's
> > > > > > > >> >> >> > >> supported
> > > > > > > >> >> >> > >> >> features as well as the cluster's finalized
> > > > features)
> > > > > > > >> >> >> > >> >>
> > > > > > > >> >> >> > >> >> --------
> > > > > > > >> >> >> > >> >>
> > > > > > > >> >> >> > >> >> Jun:
> > > > > > > >> >> >> > >> >>
> > > > > > > >> >> >> > >> >> 12. I've updated the KIP with AdminClient
> > changes
> > > > > > > >> >> >> > >> >>
> > > > > > > >> >> >> > >> >> 14. You're right, it looks like I missed a few
> > > > > sections
> > > > > > > >> >> regarding
> > > > > > > >> >> >> > >> snapshot
> > > > > > > >> >> >> > >> >> generation. I've corrected it
> > > > > > > >> >> >> > >> >>
> > > > > > > >> >> >> > >> >> 16. This feels more like an enhancement to
> > > KIP-584.
> > > > I
> > > > > > > agree
> > > > > > > >> it
> > > > > > > >> >> >> could
> > > > > > > >> >> >> > be
> > > > > > > >> >> >> > >> >> useful, but perhaps we could address it
> > separately
> > > > > from
> > > > > > > >> KRaft
> > > > > > > >> >> >> > upgrades?
> > > > > > > >> >> >> > >> >>
> > > > > > > >> >> >> > >> >> 20. Indeed snapshots are not strictly
> necessary
> > > > during
> > > > > > an
> > > > > > > >> >> upgrade,
> > > > > > > >> >> >> > I've
> > > > > > > >> >> >> > >> >> reworded this
> > > > > > > >> >> >> > >> >>
> > > > > > > >> >> >> > >> >>
> > > > > > > >> >> >> > >> >> Thanks!
> > > > > > > >> >> >> > >> >> David
> > > > > > > >> >> >> > >> >>
> > > > > > > >> >> >> > >> >>
> > > > > > > >> >> >> > >> >> On Thu, Nov 4, 2021 at 6:51 PM Jun Rao
> > > > > > > >> >> <j...@confluent.io.invalid>
> > > > > > > >> >> >> > >> wrote:
> > > > > > > >> >> >> > >> >>
> > > > > > > >> >> >> > >> >> > Hi, David, Jose and Colin,
> > > > > > > >> >> >> > >> >> >
> > > > > > > >> >> >> > >> >> > Thanks for the reply. A few more comments.
> > > > > > > >> >> >> > >> >> >
> > > > > > > >> >> >> > >> >> > 12. It seems that we haven't updated the
> > > > AdminClient
> > > > > > > >> >> accordingly?
> > > > > > > >> >> >> > >> >> >
> > > > > > > >> >> >> > >> >> > 14. "Metadata snapshot is generated and sent
> > to
> > > > the
> > > > > > > other
> > > > > > > >> >> >> inactive
> > > > > > > >> >> >> > >> >> > controllers and to brokers". I thought we
> > wanted
> > > > > each
> > > > > > > >> broker
> > > > > > > >> >> to
> > > > > > > >> >> >> > >> generate
> > > > > > > >> >> >> > >> >> > its own snapshot independently? If only the
> > > > > controller
> > > > > > > >> >> generates
> > > > > > > >> >> >> > the
> > > > > > > >> >> >> > >> >> > snapshot, how do we force other brokers to
> > pick
> > > it
> > > > > up?
> > > > > > > >> >> >> > >> >> >
> > > > > > > >> >> >> > >> >> > 16. If a feature version is new, one may not
> > > want
> > > > to
> > > > > > > >> enable
> > > > > > > >> >> it
> > > > > > > >> >> >> > >> >> immediately
> > > > > > > >> >> >> > >> >> > after the cluster is upgraded. However, if a
> > > > feature
> > > > > > > >> version
> > > > > > > >> >> has
> > > > > > > >> >> >> > been
> > > > > > > >> >> >> > >> >> > stable, requiring every user to run a
> command
> > to
> > > > > > upgrade
> > > > > > > >> to
> > > > > > > >> >> that
> > > > > > > >> >> >> > >> version
> > > > > > > >> >> >> > >> >> > seems inconvenient. One way to improve this
> is
> > > for
> > > > > > each
> > > > > > > >> >> feature
> > > > > > > >> >> >> to
> > > > > > > >> >> >> > >> define
> > > > > > > >> >> >> > >> >> > one version as the default. Then, when we
> > > upgrade
> > > > a
> > > > > > > >> cluster,
> > > > > > > >> >> we
> > > > > > > >> >> >> > will
> > > > > > > >> >> >> > >> >> > automatically upgrade the feature to the
> > default
> > > > > > > version.
> > > > > > > >> An
> > > > > > > >> >> >> admin
> > > > > > > >> >> >> > >> could
> > > > > > > >> >> >> > >> >> > use the tool to upgrade to a version higher
> > than
> > > > the
> > > > > > > >> default.
> > > > > > > >> >> >> > >> >> >
> > > > > > > >> >> >> > >> >> > 20. "The quorum controller can assist with
> > this
> > > > > > process
> > > > > > > by
> > > > > > > >> >> >> > generating
> > > > > > > >> >> >> > >> a
> > > > > > > >> >> >> > >> >> > metadata snapshot after a metadata.version
> > > > increase
> > > > > > has
> > > > > > > >> been
> > > > > > > >> >> >> > >> committed to
> > > > > > > >> >> >> > >> >> > the metadata log. This snapshot will be a
> > > > convenient
> > > > > > way
> > > > > > > >> to
> > > > > > > >> >> let
> > > > > > > >> >> >> > broker
> > > > > > > >> >> >> > >> >> and
> > > > > > > >> >> >> > >> >> > controller components rebuild their entire
> > > > in-memory
> > > > > > > state
> > > > > > > >> >> >> > following
> > > > > > > >> >> >> > >> an
> > > > > > > >> >> >> > >> >> > upgrade." The new version of the software
> > could
> > > > read
> > > > > > > both
> > > > > > > >> >> the new
> > > > > > > >> >> >> > and
> > > > > > > >> >> >> > >> the
> > > > > > > >> >> >> > >> >> > old version. Is generating a new snapshot
> > during
> > > > > > upgrade
> > > > > > > >> >> needed?
> > > > > > > >> >> >> > >> >> >
> > > > > > > >> >> >> > >> >> > Jun
> > > > > > > >> >> >> > >> >> >
> > > > > > > >> >> >> > >> >> >
> > > > > > > >> >> >> > >> >> > On Wed, Nov 3, 2021 at 5:42 PM Colin McCabe
> <
> > > > > > > >> >> cmcc...@apache.org>
> > > > > > > >> >> >> > >> wrote:
> > > > > > > >> >> >> > >> >> >
> > > > > > > >> >> >> > >> >> > > On Tue, Oct 12, 2021, at 10:34, Jun Rao
> > wrote:
> > > > > > > >> >> >> > >> >> > > > Hi, David,
> > > > > > > >> >> >> > >> >> > > >
> > > > > > > >> >> >> > >> >> > > > One more comment.
> > > > > > > >> >> >> > >> >> > > >
> > > > > > > >> >> >> > >> >> > > > 16. The main reason why KIP-584 requires
> > > > > > finalizing
> > > > > > > a
> > > > > > > >> >> feature
> > > > > > > >> >> >> > >> >> manually
> > > > > > > >> >> >> > >> >> > is
> > > > > > > >> >> >> > >> >> > > > that in the ZK world, the controller
> > doesn't
> > > > > know
> > > > > > > all
> > > > > > > >> >> brokers
> > > > > > > >> >> >> > in a
> > > > > > > >> >> >> > >> >> > > cluster.
> > > > > > > >> >> >> > >> >> > > > A broker temporarily down is not
> > registered
> > > in
> > > > > ZK.
> > > > > > > in
> > > > > > > >> the
> > > > > > > >> >> >> KRaft
> > > > > > > >> >> >> > >> >> world,
> > > > > > > >> >> >> > >> >> > > the
> > > > > > > >> >> >> > >> >> > > > controller keeps track of all brokers,
> > > > including
> > > > > > > those
> > > > > > > >> >> that
> > > > > > > >> >> >> are
> > > > > > > >> >> >> > >> >> > > temporarily
> > > > > > > >> >> >> > >> >> > > > down. This makes it possible for the
> > > > controller
> > > > > to
> > > > > > > >> >> >> > automatically
> > > > > > > >> >> >> > >> >> > > finalize a
> > > > > > > >> >> >> > >> >> > > > feature---it's safe to do so when all
> > > brokers
> > > > > > > support
> > > > > > > >> >> that
> > > > > > > >> >> >> > >> feature.
> > > > > > > >> >> >> > >> >> > This
> > > > > > > >> >> >> > >> >> > > > will make the upgrade process much
> simpler
> > > > since
> > > > > > no
> > > > > > > >> >> manual
> > > > > > > >> >> >> > >> command is
> > > > > > > >> >> >> > >> >> > > > required to turn on a new feature. Have
> we
> > > > > > > considered
> > > > > > > >> >> this?
> > > > > > > >> >> >> > >> >> > > >
> > > > > > > >> >> >> > >> >> > > > Thanks,
> > > > > > > >> >> >> > >> >> > > >
> > > > > > > >> >> >> > >> >> > > > Jun
> > > > > > > >> >> >> > >> >> > >
> > > > > > > >> >> >> > >> >> > > Hi Jun,
> > > > > > > >> >> >> > >> >> > >
> > > > > > > >> >> >> > >> >> > > I guess David commented on this point
> > already,
> > > > but
> > > > > > > I'll
> > > > > > > >> >> comment
> > > > > > > >> >> >> > as
> > > > > > > >> >> >> > >> >> well.
> > > > > > > >> >> >> > >> >> > I
> > > > > > > >> >> >> > >> >> > > always had the perception that users
> viewed
> > > > rolls
> > > > > as
> > > > > > > >> >> >> potentially
> > > > > > > >> >> >> > >> risky
> > > > > > > >> >> >> > >> >> > and
> > > > > > > >> >> >> > >> >> > > were looking for ways to reduce the risk.
> > Not
> > > > > > enabling
> > > > > > > >> >> features
> > > > > > > >> >> >> > >> right
> > > > > > > >> >> >> > >> >> > away
> > > > > > > >> >> >> > >> >> > > after installing new software seems like
> one
> > > way
> > > > > to
> > > > > > do
> > > > > > > >> >> that. If
> > > > > > > >> >> >> > we
> > > > > > > >> >> >> > >> had
> > > > > > > >> >> >> > >> >> a
> > > > > > > >> >> >> > >> >> > > feature to automatically upgrade during a
> > > roll,
> > > > > I'm
> > > > > > > not
> > > > > > > >> >> sure
> > > > > > > >> >> >> > that I
> > > > > > > >> >> >> > >> >> would
> > > > > > > >> >> >> > >> >> > > recommend that people use it, because if
> > > > something
> > > > > > > >> fails,
> > > > > > > >> >> it
> > > > > > > >> >> >> > makes
> > > > > > > >> >> >> > >> it
> > > > > > > >> >> >> > >> >> > > harder to tell if the new feature is at
> > fault,
> > > > or
> > > > > > > >> something
> > > > > > > >> >> >> else
> > > > > > > >> >> >> > in
> > > > > > > >> >> >> > >> the
> > > > > > > >> >> >> > >> >> > new
> > > > > > > >> >> >> > >> >> > > software.
> > > > > > > >> >> >> > >> >> > >
> > > > > > > >> >> >> > >> >> > > We already tell users to do a "double
> roll"
> > > when
> > > > > > going
> > > > > > > >> to
> > > > > > > >> >> a new
> > > > > > > >> >> >> > IBP.
> > > > > > > >> >> >> > >> >> > (Just
> > > > > > > >> >> >> > >> >> > > to give background to people who haven't
> > heard
> > > > > that
> > > > > > > >> >> phrase, the
> > > > > > > >> >> >> > >> first
> > > > > > > >> >> >> > >> >> > roll
> > > > > > > >> >> >> > >> >> > > installs the new software, and the second
> > roll
> > > > > > updates
> > > > > > > >> the
> > > > > > > >> >> >> IBP).
> > > > > > > >> >> >> > So
> > > > > > > >> >> >> > >> >> this
> > > > > > > >> >> >> > >> >> > > KIP-778 mechanism is basically very
> similar
> > to
> > > > > that,
> > > > > > > >> >> except the
> > > > > > > >> >> >> > >> second
> > > > > > > >> >> >> > >> >> > > thing isn't a roll, but just an upgrade
> > > command.
> > > > > So
> > > > > > I
> > > > > > > >> think
> > > > > > > >> >> >> this
> > > > > > > >> >> >> > is
> > > > > > > >> >> >> > >> >> > > consistent with what we currently do.
> > > > > > > >> >> >> > >> >> > >
> > > > > > > >> >> >> > >> >> > > Also, just like David said, we can always
> > add
> > > > > > > >> auto-upgrade
> > > > > > > >> >> >> later
> > > > > > > >> >> >> > if
> > > > > > > >> >> >> > >> >> there
> > > > > > > >> >> >> > >> >> > > is demand...
> > > > > > > >> >> >> > >> >> > >
> > > > > > > >> >> >> > >> >> > > best,
> > > > > > > >> >> >> > >> >> > > Colin
> > > > > > > >> >> >> > >> >> > >
> > > > > > > >> >> >> > >> >> > >
> > > > > > > >> >> >> > >> >> > > >
> > > > > > > >> >> >> > >> >> > > > On Thu, Oct 7, 2021 at 5:19 PM Jun Rao <
> > > > > > > >> j...@confluent.io
> > > > > > > >> >> >
> > > > > > > >> >> >> > wrote:
> > > > > > > >> >> >> > >> >> > > >
> > > > > > > >> >> >> > >> >> > > >> Hi, David,
> > > > > > > >> >> >> > >> >> > > >>
> > > > > > > >> >> >> > >> >> > > >> Thanks for the KIP. A few comments
> below.
> > > > > > > >> >> >> > >> >> > > >>
> > > > > > > >> >> >> > >> >> > > >> 10. It would be useful to describe how
> > the
> > > > > > > controller
> > > > > > > >> >> node
> > > > > > > >> >> >> > >> >> determines
> > > > > > > >> >> >> > >> >> > > the
> > > > > > > >> >> >> > >> >> > > >> RPC version used to communicate to
> other
> > > > > > controller
> > > > > > > >> >> nodes.
> > > > > > > >> >> >> > There
> > > > > > > >> >> >> > >> >> seems
> > > > > > > >> >> >> > >> >> > > to
> > > > > > > >> >> >> > >> >> > > >> be a bootstrap problem. A controller
> node
> > > > can't
> > > > > > > read
> > > > > > > >> >> the log
> > > > > > > >> >> >> > and
> > > > > > > >> >> >> > >> >> > > >> therefore the feature level until a
> > quorum
> > > > > leader
> > > > > > > is
> > > > > > > >> >> >> elected.
> > > > > > > >> >> >> > But
> > > > > > > >> >> >> > >> >> > leader
> > > > > > > >> >> >> > >> >> > > >> election requires an RPC.
> > > > > > > >> >> >> > >> >> > > >>
> > > > > > > >> >> >> > >> >> > > >> 11. For downgrades, it would be useful
> to
> > > > > > describe
> > > > > > > >> how
> > > > > > > >> >> to
> > > > > > > >> >> >> > >> determine
> > > > > > > >> >> >> > >> >> > the
> > > > > > > >> >> >> > >> >> > > >> downgrade process (generating new
> > snapshot,
> > > > > > > >> propagating
> > > > > > > >> >> the
> > > > > > > >> >> >> > >> >> snapshot,
> > > > > > > >> >> >> > >> >> > > etc)
> > > > > > > >> >> >> > >> >> > > >> has completed. We could block the
> > > > UpdateFeature
> > > > > > > >> request
> > > > > > > >> >> >> until
> > > > > > > >> >> >> > the
> > > > > > > >> >> >> > >> >> > > process
> > > > > > > >> >> >> > >> >> > > >> is completed. However, since the
> process
> > > > could
> > > > > > take
> > > > > > > >> >> time,
> > > > > > > >> >> >> the
> > > > > > > >> >> >> > >> >> request
> > > > > > > >> >> >> > >> >> > > could
> > > > > > > >> >> >> > >> >> > > >> time out. Another way is through
> > > > > DescribeFeature
> > > > > > > and
> > > > > > > >> the
> > > > > > > >> >> >> > server
> > > > > > > >> >> >> > >> only
> > > > > > > >> >> >> > >> >> > > >> reports downgraded versions after the
> > > process
> > > > > is
> > > > > > > >> >> completed.
> > > > > > > >> >> >> > >> >> > > >>
> > > > > > > >> >> >> > >> >> > > >> 12. Since we are changing
> > > > > UpdateFeaturesRequest,
> > > > > > do
> > > > > > > >> we
> > > > > > > >> >> need
> > > > > > > >> >> >> to
> > > > > > > >> >> >> > >> >> change
> > > > > > > >> >> >> > >> >> > > the
> > > > > > > >> >> >> > >> >> > > >> AdminClient api for updateFeatures too?
> > > > > > > >> >> >> > >> >> > > >>
> > > > > > > >> >> >> > >> >> > > >> 13. For the paragraph starting with "In
> > the
> > > > > > absence
> > > > > > > >> of
> > > > > > > >> >> an
> > > > > > > >> >> >> > >> operator
> > > > > > > >> >> >> > >> >> > > >> defined value for metadata.version", in
> > > > > KIP-584,
> > > > > > we
> > > > > > > >> >> >> described
> > > > > > > >> >> >> > >> how to
> > > > > > > >> >> >> > >> >> > > >> finalize features with New cluster
> > > bootstrap.
> > > > > In
> > > > > > > that
> > > > > > > >> >> case,
> > > > > > > >> >> >> > it's
> > > > > > > >> >> >> > >> >> > > >> inconvenient for the users to have to
> run
> > > an
> > > > > > admin
> > > > > > > >> tool
> > > > > > > >> >> to
> > > > > > > >> >> >> > >> finalize
> > > > > > > >> >> >> > >> >> > the
> > > > > > > >> >> >> > >> >> > > >> version for each feature. Instead, the
> > > system
> > > > > > > detects
> > > > > > > >> >> that
> > > > > > > >> >> >> the
> > > > > > > >> >> >> > >> >> > /features
> > > > > > > >> >> >> > >> >> > > >> path is missing in ZK and thus
> > > automatically
> > > > > > > >> finalizes
> > > > > > > >> >> every
> > > > > > > >> >> >> > >> feature
> > > > > > > >> >> >> > >> >> > > with
> > > > > > > >> >> >> > >> >> > > >> the latest supported version. Could we
> do
> > > > > > something
> > > > > > > >> >> similar
> > > > > > > >> >> >> in
> > > > > > > >> >> >> > >> the
> > > > > > > >> >> >> > >> >> > KRaft
> > > > > > > >> >> >> > >> >> > > >> mode?
> > > > > > > >> >> >> > >> >> > > >>
> > > > > > > >> >> >> > >> >> > > >> 14. After the quorum leader generates a
> > new
> > > > > > > snapshot,
> > > > > > > >> >> how do
> > > > > > > >> >> >> > we
> > > > > > > >> >> >> > >> >> force
> > > > > > > >> >> >> > >> >> > > >> other nodes to pick up the new
> snapshot?
> > > > > > > >> >> >> > >> >> > > >>
> > > > > > > >> >> >> > >> >> > > >> 15. I agree with Jose that it will be
> > > useful
> > > > to
> > > > > > > >> describe
> > > > > > > >> >> >> when
> > > > > > > >> >> >> > >> >> > > generating a
> > > > > > > >> >> >> > >> >> > > >> new snapshot is needed. To me, it seems
> > the
> > > > new
> > > > > > > >> >> snapshot is
> > > > > > > >> >> >> > only
> > > > > > > >> >> >> > >> >> > needed
> > > > > > > >> >> >> > >> >> > > >> when incompatible changes are made.
> > > > > > > >> >> >> > >> >> > > >>
> > > > > > > >> >> >> > >> >> > > >> 7. Jose, what control records were you
> > > > > referring?
> > > > > > > >> >> >> > >> >> > > >>
> > > > > > > >> >> >> > >> >> > > >> Thanks,
> > > > > > > >> >> >> > >> >> > > >>
> > > > > > > >> >> >> > >> >> > > >> Jun
> > > > > > > >> >> >> > >> >> > > >>
> > > > > > > >> >> >> > >> >> > > >>
> > > > > > > >> >> >> > >> >> > > >> On Tue, Oct 5, 2021 at 8:53 AM David
> > > Arthur <
> > > > > > > >> >> >> > >> davidart...@apache.org
> > > > > > > >> >> >> > >> >> >
> > > > > > > >> >> >> > >> >> > > >> wrote:
> > > > > > > >> >> >> > >> >> > > >>
> > > > > > > >> >> >> > >> >> > > >>> Jose, thanks for the thorough review
> and
> > > > > > comments!
> > > > > > > >> >> >> > >> >> > > >>>
> > > > > > > >> >> >> > >> >> > > >>> I am out of the office until next
> week,
> > > so I
> > > > > > > >> probably
> > > > > > > >> >> won't
> > > > > > > >> >> >> > be
> > > > > > > >> >> >> > >> able
> > > > > > > >> >> >> > >> >> > to
> > > > > > > >> >> >> > >> >> > > >>> update the KIP until then. Here are
> some
> > > > > replies
> > > > > > > to
> > > > > > > >> >> your
> > > > > > > >> >> >> > >> questions:
> > > > > > > >> >> >> > >> >> > > >>>
> > > > > > > >> >> >> > >> >> > > >>> 1. Generate snapshot on upgrade
> > > > > > > >> >> >> > >> >> > > >>> > > Metadata snapshot is generated and
> > > sent
> > > > to
> > > > > > the
> > > > > > > >> >> other
> > > > > > > >> >> >> > nodes
> > > > > > > >> >> >> > >> >> > > >>> > Why does the Active Controller need
> to
> > > > > > generate
> > > > > > > a
> > > > > > > >> new
> > > > > > > >> >> >> > snapshot
> > > > > > > >> >> >> > >> >> and
> > > > > > > >> >> >> > >> >> > > >>> > force a snapshot fetch from the
> > replicas
> > > > > > > (inactive
> > > > > > > >> >> >> > controller
> > > > > > > >> >> >> > >> and
> > > > > > > >> >> >> > >> >> > > >>> > brokers) on an upgrade? Isn't
> writing
> > > the
> > > > > > > >> >> >> > FeatureLevelRecord
> > > > > > > >> >> >> > >> good
> > > > > > > >> >> >> > >> >> > > >>> > enough to communicate the upgrade to
> > the
> > > > > > > replicas?
> > > > > > > >> >> >> > >> >> > > >>>
> > > > > > > >> >> >> > >> >> > > >>>
> > > > > > > >> >> >> > >> >> > > >>> You're right, we don't necessarily
> need
> > to
> > > > > > > >> _transmit_ a
> > > > > > > >> >> >> > >> snapshot,
> > > > > > > >> >> >> > >> >> > since
> > > > > > > >> >> >> > >> >> > > >>> each node can generate its own
> > equivalent
> > > > > > snapshot
> > > > > > > >> >> >> > >> >> > > >>>
> > > > > > > >> >> >> > >> >> > > >>> 2. Generate snapshot on downgrade
> > > > > > > >> >> >> > >> >> > > >>> > > Metadata snapshot is generated and
> > > sent
> > > > to
> > > > > > the
> > > > > > > >> >> other
> > > > > > > >> >> >> > >> inactive
> > > > > > > >> >> >> > >> >> > > >>> > controllers and to brokers (this
> > > snapshot
> > > > > may
> > > > > > be
> > > > > > > >> >> lossy!)
> > > > > > > >> >> >> > >> >> > > >>> > Why do we need to send this
> downgraded
> > > > > > snapshot
> > > > > > > to
> > > > > > > >> >> the
> > > > > > > >> >> >> > >> brokers?
> > > > > > > >> >> >> > >> >> The
> > > > > > > >> >> >> > >> >> > > >>> > replicas have seen the
> > > FeatureLevelRecord
> > > > > and
> > > > > > > >> >> noticed the
> > > > > > > >> >> >> > >> >> > downgrade.
> > > > > > > >> >> >> > >> >> > > >>> > Can we have the replicas each
> > > > independently
> > > > > > > >> generate
> > > > > > > >> >> a
> > > > > > > >> >> >> > >> downgraded
> > > > > > > >> >> >> > >> >> > > >>> > snapshot at the offset for the
> > > downgraded
> > > > > > > >> >> >> > FeatureLevelRecord?
> > > > > > > >> >> >> > >> I
> > > > > > > >> >> >> > >> >> > > assume
> > > > > > > >> >> >> > >> >> > > >>> > that the active controller will
> > > guarantee
> > > > > that
> > > > > > > all
> > > > > > > >> >> >> records
> > > > > > > >> >> >> > >> after
> > > > > > > >> >> >> > >> >> > the
> > > > > > > >> >> >> > >> >> > > >>> > FatureLevelRecord use the downgraded
> > > > > version.
> > > > > > If
> > > > > > > >> so,
> > > > > > > >> >> it
> > > > > > > >> >> >> > would
> > > > > > > >> >> >> > >> be
> > > > > > > >> >> >> > >> >> > good
> > > > > > > >> >> >> > >> >> > > >>> > to mention that explicitly.
> > > > > > > >> >> >> > >> >> > > >>>
> > > > > > > >> >> >> > >> >> > > >>>
> > > > > > > >> >> >> > >> >> > > >>> Similar to above, yes a broker that
> > > detects
> > > > a
> > > > > > > >> >> downgrade via
> > > > > > > >> >> >> > >> >> > > >>> FeatureLevelRecord could generate its
> > own
> > > > > > > downgrade
> > > > > > > >> >> >> snapshot
> > > > > > > >> >> >> > and
> > > > > > > >> >> >> > >> >> > reload
> > > > > > > >> >> >> > >> >> > > >>> its
> > > > > > > >> >> >> > >> >> > > >>> state from that. This does get a
> little
> > > > fuzzy
> > > > > > when
> > > > > > > >> we
> > > > > > > >> >> >> > consider
> > > > > > > >> >> >> > >> >> cases
> > > > > > > >> >> >> > >> >> > > where
> > > > > > > >> >> >> > >> >> > > >>> brokers are on different software
> > versions
> > > > and
> > > > > > > >> could be
> > > > > > > >> >> >> > >> generating
> > > > > > > >> >> >> > >> >> a
> > > > > > > >> >> >> > >> >> > > >>> downgrade snapshot for version X, but
> > > using
> > > > > > > >> different
> > > > > > > >> >> >> > versions
> > > > > > > >> >> >> > >> of
> > > > > > > >> >> >> > >> >> the
> > > > > > > >> >> >> > >> >> > > >>> code.
> > > > > > > >> >> >> > >> >> > > >>> It might be safer to let the
> controller
> > > > > generate
> > > > > > > the
> > > > > > > >> >> >> > snapshot so
> > > > > > > >> >> >> > >> >> each
> > > > > > > >> >> >> > >> >> > > >>> broker (regardless of software
> version)
> > > gets
> > > > > the
> > > > > > > >> same
> > > > > > > >> >> >> > records.
> > > > > > > >> >> >> > >> >> > However,
> > > > > > > >> >> >> > >> >> > > >>> for
> > > > > > > >> >> >> > >> >> > > >>> upgrades (or downgrades) we expect the
> > > whole
> > > > > > > cluster
> > > > > > > >> >> to be
> > > > > > > >> >> >> > >> running
> > > > > > > >> >> >> > >> >> > the
> > > > > > > >> >> >> > >> >> > > >>> same
> > > > > > > >> >> >> > >> >> > > >>> software version before triggering the
> > > > > > > >> metadata.version
> > > > > > > >> >> >> > change,
> > > > > > > >> >> >> > >> so
> > > > > > > >> >> >> > >> >> > > perhaps
> > > > > > > >> >> >> > >> >> > > >>> this isn't a likely scenario.
> Thoughts?
> > > > > > > >> >> >> > >> >> > > >>>
> > > > > > > >> >> >> > >> >> > > >>>
> > > > > > > >> >> >> > >> >> > > >>> 3. Max metadata version
> > > > > > > >> >> >> > >> >> > > >>> > >For the first release that supports
> > > > > > > >> >> metadata.version, we
> > > > > > > >> >> >> > can
> > > > > > > >> >> >> > >> >> > simply
> > > > > > > >> >> >> > >> >> > > >>> > initialize metadata.version with the
> > > > current
> > > > > > > (and
> > > > > > > >> >> only)
> > > > > > > >> >> >> > >> version.
> > > > > > > >> >> >> > >> >> > For
> > > > > > > >> >> >> > >> >> > > >>> future
> > > > > > > >> >> >> > >> >> > > >>> > releases, we will need a mechanism
> to
> > > > > > bootstrap
> > > > > > > a
> > > > > > > >> >> >> > particular
> > > > > > > >> >> >> > >> >> > version.
> > > > > > > >> >> >> > >> >> > > >>> This
> > > > > > > >> >> >> > >> >> > > >>> > could be done using the
> > meta.properties
> > > > file
> > > > > > or
> > > > > > > >> some
> > > > > > > >> >> >> > similar
> > > > > > > >> >> >> > >> >> > > mechanism.
> > > > > > > >> >> >> > >> >> > > >>> The
> > > > > > > >> >> >> > >> >> > > >>> > reason we need the allow for a
> > specific
> > > > > > initial
> > > > > > > >> >> version
> > > > > > > >> >> >> is
> > > > > > > >> >> >> > to
> > > > > > > >> >> >> > >> >> > support
> > > > > > > >> >> >> > >> >> > > >>> the
> > > > > > > >> >> >> > >> >> > > >>> > use case of starting a Kafka cluster
> > at
> > > > > > version
> > > > > > > X
> > > > > > > >> >> with an
> > > > > > > >> >> >> > >> older
> > > > > > > >> >> >> > >> >> > > >>> > metadata.version.
> > > > > > > >> >> >> > >> >> > > >>>
> > > > > > > >> >> >> > >> >> > > >>>
> > > > > > > >> >> >> > >> >> > > >>> I assume that the Active Controller
> will
> > > > learn
> > > > > > the
> > > > > > > >> >> metadata
> > > > > > > >> >> >> > >> version
> > > > > > > >> >> >> > >> >> > of
> > > > > > > >> >> >> > >> >> > > >>> > the broker through the
> > > > > > > BrokerRegistrationRequest.
> > > > > > > >> How
> > > > > > > >> >> >> will
> > > > > > > >> >> >> > the
> > > > > > > >> >> >> > >> >> > Active
> > > > > > > >> >> >> > >> >> > > >>> > Controller learn about the max
> > metadata
> > > > > > version
> > > > > > > of
> > > > > > > >> >> the
> > > > > > > >> >> >> > >> inactive
> > > > > > > >> >> >> > >> >> > > >>> > controller nodes? We currently don't
> > > send
> > > > a
> > > > > > > >> >> registration
> > > > > > > >> >> >> > >> request
> > > > > > > >> >> >> > >> >> > from
> > > > > > > >> >> >> > >> >> > > >>> > the inactive controller to the
> active
> > > > > > > controller.
> > > > > > > >> >> >> > >> >> > > >>>
> > > > > > > >> >> >> > >> >> > > >>>
> > > > > > > >> >> >> > >> >> > > >>> This came up during the design, but I
> > > > > neglected
> > > > > > to
> > > > > > > >> add
> > > > > > > >> >> it
> > > > > > > >> >> >> to
> > > > > > > >> >> >> > the
> > > > > > > >> >> >> > >> >> KIP.
> > > > > > > >> >> >> > >> >> > > We
> > > > > > > >> >> >> > >> >> > > >>> will need a mechanism for determining
> > the
> > > > > > > supported
> > > > > > > >> >> >> features
> > > > > > > >> >> >> > of
> > > > > > > >> >> >> > >> >> each
> > > > > > > >> >> >> > >> >> > > >>> controller similar to how brokers use
> > > > > > > >> >> >> > BrokerRegistrationRequest.
> > > > > > > >> >> >> > >> >> > > Perhaps
> > > > > > > >> >> >> > >> >> > > >>> controllers could write a
> > > FeatureLevelRecord
> > > > > (or
> > > > > > > >> >> similar)
> > > > > > > >> >> >> to
> > > > > > > >> >> >> > the
> > > > > > > >> >> >> > >> >> > > metadata
> > > > > > > >> >> >> > >> >> > > >>> log indicating their supported
> version.
> > > > WDYT?
> > > > > > > >> >> >> > >> >> > > >>>
> > > > > > > >> >> >> > >> >> > > >>> Why do you need to bootstrap a
> > particular
> > > > > > version?
> > > > > > > >> >> Isn't
> > > > > > > >> >> >> the
> > > > > > > >> >> >> > >> intent
> > > > > > > >> >> >> > >> >> > > >>> > that the broker will learn the
> active
> > > > > metadata
> > > > > > > >> >> version by
> > > > > > > >> >> >> > >> reading
> > > > > > > >> >> >> > >> >> > the
> > > > > > > >> >> >> > >> >> > > >>> > metadata before unfencing?
> > > > > > > >> >> >> > >> >> > > >>>
> > > > > > > >> >> >> > >> >> > > >>>
> > > > > > > >> >> >> > >> >> > > >>> This bootstrapping is needed for when
> a
> > > > KRaft
> > > > > > > >> cluster
> > > > > > > >> >> is
> > > > > > > >> >> >> > first
> > > > > > > >> >> >> > >> >> > > started. If
> > > > > > > >> >> >> > >> >> > > >>> we don't have this mechanism, the
> > cluster
> > > > > can't
> > > > > > > >> really
> > > > > > > >> >> do
> > > > > > > >> >> >> > >> anything
> > > > > > > >> >> >> > >> >> > > until
> > > > > > > >> >> >> > >> >> > > >>> the operator finalizes the
> > > metadata.version
> > > > > with
> > > > > > > the
> > > > > > > >> >> tool.
> > > > > > > >> >> >> > The
> > > > > > > >> >> >> > >> >> > > >>> bootstrapping will be done by the
> > > controller
> > > > > and
> > > > > > > the
> > > > > > > >> >> >> brokers
> > > > > > > >> >> >> > >> will
> > > > > > > >> >> >> > >> >> see
> > > > > > > >> >> >> > >> >> > > this
> > > > > > > >> >> >> > >> >> > > >>> version as a record (like you say).
> I'll
> > > add
> > > > > > some
> > > > > > > >> text
> > > > > > > >> >> to
> > > > > > > >> >> >> > >> clarify
> > > > > > > >> >> >> > >> >> > this.
> > > > > > > >> >> >> > >> >> > > >>>
> > > > > > > >> >> >> > >> >> > > >>>
> > > > > > > >> >> >> > >> >> > > >>> 4. Reject Registration - This is
> related
> > > to
> > > > > the
> > > > > > > >> bullet
> > > > > > > >> >> >> point
> > > > > > > >> >> >> > >> above.
> > > > > > > >> >> >> > >> >> > > >>> > What will be the behavior of the
> > active
> > > > > > > controller
> > > > > > > >> >> if the
> > > > > > > >> >> >> > >> broker
> > > > > > > >> >> >> > >> >> > > sends
> > > > > > > >> >> >> > >> >> > > >>> > a metadata version that is not
> > > compatible
> > > > > with
> > > > > > > the
> > > > > > > >> >> >> cluster
> > > > > > > >> >> >> > >> wide
> > > > > > > >> >> >> > >> >> > > >>> > metadata version?
> > > > > > > >> >> >> > >> >> > > >>>
> > > > > > > >> >> >> > >> >> > > >>>
> > > > > > > >> >> >> > >> >> > > >>> If a broker starts up with a lower
> > > supported
> > > > > > > version
> > > > > > > >> >> range
> > > > > > > >> >> >> > than
> > > > > > > >> >> >> > >> the
> > > > > > > >> >> >> > >> >> > > >>> current
> > > > > > > >> >> >> > >> >> > > >>> cluster metadata.version, it should
> log
> > an
> > > > > error
> > > > > > > and
> > > > > > > >> >> >> > shutdown.
> > > > > > > >> >> >> > >> This
> > > > > > > >> >> >> > >> >> > is
> > > > > > > >> >> >> > >> >> > > in
> > > > > > > >> >> >> > >> >> > > >>> line with KIP-584.
> > > > > > > >> >> >> > >> >> > > >>>
> > > > > > > >> >> >> > >> >> > > >>> 5. Discover upgrade
> > > > > > > >> >> >> > >> >> > > >>> > > This snapshot will be a convenient
> > way
> > > > to
> > > > > > let
> > > > > > > >> >> broker
> > > > > > > >> >> >> and
> > > > > > > >> >> >> > >> >> > controller
> > > > > > > >> >> >> > >> >> > > >>> > components rebuild their entire
> > > in-memory
> > > > > > state
> > > > > > > >> >> following
> > > > > > > >> >> >> > an
> > > > > > > >> >> >> > >> >> > upgrade.
> > > > > > > >> >> >> > >> >> > > >>> > Can we rely on the presence of the
> > > > > > > >> >> FeatureLevelRecord for
> > > > > > > >> >> >> > the
> > > > > > > >> >> >> > >> >> > > metadata
> > > > > > > >> >> >> > >> >> > > >>> > version for this functionality? If
> so,
> > > it
> > > > > > avoids
> > > > > > > >> >> having
> > > > > > > >> >> >> to
> > > > > > > >> >> >> > >> reload
> > > > > > > >> >> >> > >> >> > the
> > > > > > > >> >> >> > >> >> > > >>> > snapshot.
> > > > > > > >> >> >> > >> >> > > >>>
> > > > > > > >> >> >> > >> >> > > >>>
> > > > > > > >> >> >> > >> >> > > >>> For upgrades, yes probably since we
> > won't
> > > > need
> > > > > > to
> > > > > > > >> >> "rewrite"
> > > > > > > >> >> >> > any
> > > > > > > >> >> >> > >> >> > > records in
> > > > > > > >> >> >> > >> >> > > >>> this case. For downgrades, we will
> need
> > to
> > > > > > > generate
> > > > > > > >> the
> > > > > > > >> >> >> > snapshot
> > > > > > > >> >> >> > >> >> and
> > > > > > > >> >> >> > >> >> > > >>> reload
> > > > > > > >> >> >> > >> >> > > >>> everything.
> > > > > > > >> >> >> > >> >> > > >>>
> > > > > > > >> >> >> > >> >> > > >>> 6. Metadata version specification
> > > > > > > >> >> >> > >> >> > > >>> > > V4(version=4,
> > > > > isBackwardsCompatible=false,
> > > > > > > >> >> >> > description="New
> > > > > > > >> >> >> > >> >> > > metadata
> > > > > > > >> >> >> > >> >> > > >>> > record type Bar"),
> > > > > > > >> >> >> > >> >> > > >>>
> > > > > > > >> >> >> > >> >> > > >>>
> > > > > > > >> >> >> > >> >> > > >>> Very cool. Do you have plans to
> generate
> > > > > Apache
> > > > > > > >> Kafka
> > > > > > > >> >> HTML
> > > > > > > >> >> >> > >> >> > > >>> > documentation for this information?
> > > Would
> > > > be
> > > > > > > >> helpful
> > > > > > > >> >> to
> > > > > > > >> >> >> > >> display
> > > > > > > >> >> >> > >> >> > this
> > > > > > > >> >> >> > >> >> > > >>> > information to the user using the
> > > > > > > >> kafka-features.sh
> > > > > > > >> >> and
> > > > > > > >> >> >> > >> feature
> > > > > > > >> >> >> > >> >> > RPC?
> > > > > > > >> >> >> > >> >> > > >>>
> > > > > > > >> >> >> > >> >> > > >>>
> > > > > > > >> >> >> > >> >> > > >>> Hm good idea :) I'll add a brief
> section
> > > on
> > > > > > > >> >> documentation.
> > > > > > > >> >> >> > This
> > > > > > > >> >> >> > >> >> would
> > > > > > > >> >> >> > >> >> > > >>> certainly be very useful
> > > > > > > >> >> >> > >> >> > > >>>
> > > > > > > >> >> >> > >> >> > > >>> 7.Downgrade records
> > > > > > > >> >> >> > >> >> > > >>> > I think we should explicitly mention
> > > that
> > > > > the
> > > > > > > >> >> downgrade
> > > > > > > >> >> >> > >> process
> > > > > > > >> >> >> > >> >> > will
> > > > > > > >> >> >> > >> >> > > >>> > downgrade both metadata records and
> > > > > controller
> > > > > > > >> >> records.
> > > > > > > >> >> >> In
> > > > > > > >> >> >> > >> >> KIP-630
> > > > > > > >> >> >> > >> >> > we
> > > > > > > >> >> >> > >> >> > > >>> > introduced two control records for
> > > > > snapshots.
> > > > > > > >> >> >> > >> >> > > >>>
> > > > > > > >> >> >> > >> >> > > >>>
> > > > > > > >> >> >> > >> >> > > >>> Yes, good call. Let me re-read that
> KIP
> > > and
> > > > > > > include
> > > > > > > >> >> some
> > > > > > > >> >> >> > >> details.
> > > > > > > >> >> >> > >> >> > > >>>
> > > > > > > >> >> >> > >> >> > > >>> Thanks again for the comments!
> > > > > > > >> >> >> > >> >> > > >>> -David
> > > > > > > >> >> >> > >> >> > > >>>
> > > > > > > >> >> >> > >> >> > > >>>
> > > > > > > >> >> >> > >> >> > > >>> On Wed, Sep 29, 2021 at 5:09 PM José
> > > Armando
> > > > > > > García
> > > > > > > >> >> Sancio
> > > > > > > >> >> >> > >> >> > > >>> <jsan...@confluent.io.invalid> wrote:
> > > > > > > >> >> >> > >> >> > > >>>
> > > > > > > >> >> >> > >> >> > > >>> > One more comment.
> > > > > > > >> >> >> > >> >> > > >>> >
> > > > > > > >> >> >> > >> >> > > >>> > 7.Downgrade records
> > > > > > > >> >> >> > >> >> > > >>> > I think we should explicitly mention
> > > that
> > > > > the
> > > > > > > >> >> downgrade
> > > > > > > >> >> >> > >> process
> > > > > > > >> >> >> > >> >> > will
> > > > > > > >> >> >> > >> >> > > >>> > downgrade both metadata records and
> > > > > controller
> > > > > > > >> >> records.
> > > > > > > >> >> >> In
> > > > > > > >> >> >> > >> >> KIP-630
> > > > > > > >> >> >> > >> >> > we
> > > > > > > >> >> >> > >> >> > > >>> > introduced two control records for
> > > > > snapshots.
> > > > > > > >> >> >> > >> >> > > >>> >
> > > > > > > >> >> >> > >> >> > > >>>
> > > > > > > >> >> >> > >> >> > > >>
> > > > > > > >> >> >> > >> >> > >
> > > > > > > >> >> >> > >> >> >
> > > > > > > >> >> >> > >> >>
> > > > > > > >> >> >> > >> >>
> > > > > > > >> >> >> > >> >> --
> > > > > > > >> >> >> > >> >> -David
> > > > > > > >> >> >> > >> >>
> > > > > > > >> >> >> > >>
> > > > > > > >> >> >> >
> > > > > > > >> >> >>
> > > > > > > >> >>
> > > > > > > >> >
> > > > > > > >>
> > > > > > > >
> > > > > > > >
> > > > > > > > --
> > > > > > > > -David
> > > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > --
> > > > > > > -David
> > > > > > >
> > > > > >
> > > > >
> > > > >
> > > > > --
> > > > > -David
> > > > >
> > > >
> > >
> > >
> > > --
> > > -David
> > >
> >
>
>
> --
> -David
>
