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