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é > > > > > >> > > > > > > > > > > > > > > >
