Hey Jun,

Right, sorry this would be one row in an output of all the various features
(transaction.protocol.version, group.coordinator.version) currently set on
the cluster.

If we want to know the dependencies for each of a given feature (ie
transaction.protocol.versions 0, 1, 2, 3 etc) we can use the other command
if those versions are not yet set.

If this makes sense I will update the KIP.

I think one remaining open area was with respect to  `--release-version`
I didn't follow Colin's point
> So how does the command invoking --release-version know whether it's
upgrading or downgrading?
The feature command includes the upgrade or downgrade command along with
the --release-version flag. If some features are not moving in the
direction mentioned (upgrade or downgrade) the command will fail -- perhaps
with an error of which features were going in the wrong direction.

Thanks,
Justine

On Wed, Mar 27, 2024 at 11:52 AM Jun Rao <j...@confluent.io.invalid> wrote:

> Hi, Justine,
>
> It seems that we need to specify the dependencies for each feature version?
>
> Thanks,
>
> Jun
>
> On Wed, Mar 27, 2024 at 11:28 AM Justine Olshan
> <jols...@confluent.io.invalid> wrote:
>
> > Hey Jun,
> >
> > So just including the dependencies for the currently set features? Along
> > with the supported min, max, and finalized versions?
> >
> > Feature: transaction.protocol.version SupportedMinVersion: 0
> > SupportedMaxVersion: 2 FinalizedVersionLevel: 1 Epoch: 3 Dependencies:
> > metadata.version=4
> >
> > On Wed, Mar 27, 2024 at 11:14 AM Jun Rao <j...@confluent.io.invalid>
> wrote:
> >
> > > Hi, Justine,
> > >
> > > Yes, something like that. We could also extend "kafka-feature describe"
> > by
> > > adding the dependency to every feature in the output.
> > >
> > > Thanks,
> > >
> > > Jun
> > >
> > > On Wed, Mar 27, 2024 at 10:39 AM Justine Olshan
> > > <jols...@confluent.io.invalid> wrote:
> > >
> > > > Hi Jun,
> > > >
> > > > We could expose them in a tool. I'm wondering, are you thinking
> > something
> > > > like a command that lists the dependencies for a given feature +
> > version?
> > > >
> > > > Something like:
> > > > kafka-feature dependencies --feature transaction.protocol.version=2
> > > > > transaction.protocol.verison=2 requires metadata.version=4 (listing
> > any
> > > > other version dependencies)
> > > >
> > > > Justine
> > > >
> > > > On Wed, Mar 27, 2024 at 10:28 AM Jun Rao <j...@confluent.io.invalid>
> > > wrote:
> > > >
> > > > > Hi, Colin,
> > > > >
> > > > > Thanks for the comments. It's fine if we want to keep the
> --metadata
> > > flag
> > > > > if it's useful. Then we should add the same flag for kafka-storage
> > for
> > > > > consistency, right?
> > > > >
> > > > > Hi, Justine,
> > > > >
> > > > > Thanks for the reply.
> > > > >
> > > > > How will a user know about the dependencies among features? Should
> we
> > > > > expose them in a tool?
> > > > >
> > > > > Thanks,
> > > > >
> > > > > Jun
> > > > >
> > > > > On Tue, Mar 26, 2024 at 4:33 PM Colin McCabe <cmcc...@apache.org>
> > > wrote:
> > > > >
> > > > > > Hi all,
> > > > > >
> > > > > > Thanks for the discussion. Let me respond to a few questions I
> saw
> > in
> > > > the
> > > > > > thread (hope I didn't miss any!)
> > > > > >
> > > > > > ============
> > > > > >
> > > > > > Feature level 0 is "special" because it effectively means that
> the
> > > > > feature
> > > > > > hasn't been set up. So I could create a foo feature, a bar
> feature,
> > > > and a
> > > > > > baz feature tomorrow and correctly say that your cluster is on
> > > feature
> > > > > > level 0 for foo, bar, and baz. Because feature level 0 is
> > isomorphic
> > > > with
> > > > > > "no feature level set."
> > > > > >
> > > > > > Obviously you can have whatever semantics you want for feature
> > level
> > > 0.
> > > > > In
> > > > > > the case of Jose's Raft changes, feature level 0 may end up being
> > the
> > > > > > current state of the world. That's fine.
> > > > > >
> > > > > > 0 being isomorphic with "not set" simplifes the code a lot
> because
> > we
> > > > > > don't need tons of special cases for "feature not set" versus
> > > "feature
> > > > > set
> > > > > > to 0". Effectively we can use short integers everywhere, and not
> > > > > > Optional<Short>. Does that make sense?
> > > > > >
> > > > > > ============
> > > > > >
> > > > > > The --metadata flag doesn't quite do the same thing as the
> > --feature
> > > > > flag.
> > > > > > The --metadata flag takes a string like 3.7-IV0, whereas the
> > > --feature
> > > > > flag
> > > > > > takes an integer like "17".
> > > > > >
> > > > > > It's true that in the current kafka-features.sh, you can shun the
> > > > > > --metadata flag, and only use --feature. The --metadata flag is a
> > > > > > convenience. But... conveniences are good. Better than
> > > inconveniences.
> > > > > So I
> > > > > > don't think it makes sense to deprecate --metadata, since its
> > > > > functionality
> > > > > > is not provided by anything else.
> > > > > >
> > > > > > ============
> > > > > >
> > > > > > As I said earlier, the proposed semantics for --release-version
> > > aren't
> > > > > > actually possible given the current RPCs on the server side. The
> > > > problem
> > > > > is
> > > > > > that UpdateFeaturesRequest needs to be set upgradType to one of:
> > > > > >
> > > > > > 1. FeatureUpdate.UpgradeType.UPGRADE
> > > > > > 2. FeatureUpdate.UpgradeType.SAFE_DOWNGRADE
> > > > > > 3. FeatureUpdate.UpgradeType.UNSAFE_DOWNGRADE
> > > > > >
> > > > > > If it's set to #1, levels can only go up; if it's set to 2 or 3,
> > > levels
> > > > > > can only go down. (I forget what happens if the levels are the
> > > same...
> > > > > you
> > > > > > can check)
> > > > > >
> > > > > > So how does the command invoking --release-version know whether
> > it's
> > > > > > upgrading or downgrading? I can't see any way for it to know, and
> > > plus
> > > > it
> > > > > > may end up doing more than one of these if some features need to
> go
> > > > down
> > > > > > and others up. "Making everything the same as it was in 3.7-IV0"
> > may
> > > > end
> > > > > up
> > > > > > down-levelling some features, and up-levelling others. There
> isn't
> > > any
> > > > > way
> > > > > > to do this atomically in a single RPC today.
> > > > > >
> > > > > > ============
> > > > > >
> > > > > > I don't find the proposed semantics for --release-version to be
> > very
> > > > > > useful.
> > > > > >
> > > > > > In the clusters I help to administer, I don't like changing a
> bunch
> > > of
> > > > > > things all at once. I'd much rather make one change at a time and
> > see
> > > > > what
> > > > > > happens, then move on to the next change.
> > > > > >
> > > > > > Earlier I proposed just having a subcommand in kafka-features.sh
> > that
> > > > > > compared the currently set feature flags against the "default"
> one
> > > for
> > > > > the
> > > > > > provided Kafka release / MV. I think this would be more useful
> than
> > > the
> > > > > > "shotgun approach" of making a bunch of changes together. Just
> > > DISPLAY
> > > > > what
> > > > > > would need to be changed to "make everything the same as it was
> in
> > > > > 3.7-IV0"
> > > > > > but then let the admin decide what they want to do (or not do).
> You
> > > > could
> > > > > > even display the commands that would need to be run, if you like.
> > But
> > > > let
> > > > > > them decide whether to pull the trigger on each upgrade or
> > downgrade.
> > > > > >
> > > > > > This also avoids having to solve the thorny issue of how to have
> a
> > > > single
> > > > > > RPC do both upgrades and downgrades.
> > > > > >
> > > > > > best,
> > > > > > Colin
> > > > > >
> > > > > >
> > > > > > On Tue, Mar 26, 2024, at 14:59, Justine Olshan wrote:
> > > > > > > Hi all,
> > > > > > >
> > > > > > > I've added the notes about requiring 3.3-IV0 and that the
> > tool/rpc
> > > > will
> > > > > > > fail if the metadata version is too low.
> > > > > > >
> > > > > > > I will wait for Colin's response on the deprecation. I am not
> > > opposed
> > > > > to
> > > > > > > deprecating it but want everyone to agree.
> > > > > > >
> > > > > > > Justine
> > > > > > >
> > > > > > > On Tue, Mar 26, 2024 at 12:26 PM José Armando García Sancio
> > > > > > > <jsan...@confluent.io.invalid> wrote:
> > > > > > >
> > > > > > >> Hi Justine,
> > > > > > >>
> > > > > > >> On Mon, Mar 25, 2024 at 5:09 PM Justine Olshan
> > > > > > >> <jols...@confluent.io.invalid> wrote:
> > > > > > >> > The reason it is not removed is purely for backwards
> > > > > > >> > compatibility. Colin had strong feelings about not removing
> > any
> > > > > flags.
> > > > > > >>
> > > > > > >> We are not saying that we should remove that flag. That would
> > > break
> > > > > > >> backward compatibility of 3.8 with 3.7. We are suggesting to
> > > > deprecate
> > > > > > >> the flag in the next release.
> > > > > > >>
> > > > > > >> Thanks,
> > > > > > >> -José
> > > > > > >>
> > > > > >
> > > > >
> > > >
> > >
> >
>

Reply via email to