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