Hi, Justine, Thanks for the reply.
11. Yes, we can still have the option to set individual features. I was suggesting that if one uses the tool without either --release-version or --features, it will just use the latest version of every feature that it knows. This is what's proposed in the original KIP-584. 12. If we can't deprecate --metadata METADATA, it would be useful to describe the reason. Jun On Thu, Feb 29, 2024 at 3:39 PM Justine Olshan <jols...@confluent.io.invalid> wrote: > Hey folks, > > Thanks for the discussion. Let me try to cover everyone's comments. > > Artem -- > I can add the examples you mentioned. As for naming, right now the feature > is called "transaction version" or "TV". Are you suggesting it should be > called "transaction protocol version" or "TPV"? I don't mind that, but just > wanted to clarify if we want to include protocol or if simply "transaction > version" is enough. > > Jun -- > > 10. *With **more features, would each of those be controlled by a separate > feature or* > > *multiple features. For example, is the new transaction record format* > > *controlled only by MV with TV having a dependency on MV or is it > controlled* > > *by both MV and TV.* > > > I think this will need to be decided on a case by case basis. There should > be a mechanism to set dependencies among features. > For transaction version specifically, I have no metadata version > dependencies besides requiring 3.3 to write the feature records and use the > feature tools. I would suspect all new features would have this > requirement. > > > 11. *Basically, if **--release-version is not used, the command will just > use the latest* > > *production version of every feature. Should we apply that logic to both* > > *tools?* > > > How would this work with the upgrade tool? I think we want a way to set a > new feature version for one feature and not touch any of the others. > > > *12. Should we remove --metadata METADATA from kafka-features? It does the* > > *same thing as --release-version.* > > > When I previously discussed with Colin McCabe offline about this tool, he > was strongly against deprecation or changing flags. I personally think it > could good > > to unify and not support a ton of flags, but I would want to make sure he > is aligned. > > > *13. KIP-853 also extends the tools to support a new feature > kraft.version.* > > *It would be useful to have alignment between that KIP and this one.* > > > Sounds good. Looks like Jose is in on the discussion so we can continue > here. :) > > > > Jose -- > > > *1. KIP-853 uses --feature for kafka-storage instead of --features.* > > *This is consistent with the use of --feature in the "kafka-feature.sh* > > *upgrade" command.* > > > I wanted to include multiple features in one command, so it seems like > features is a better name. I discuss more below about why I think we should > allow setting multiple features at once. > > > *2. I find it a bit inconsistent that --feature and --release-version* > > *are mutually exclusive in the kafka-feature CLI but not in the* > > *kafka-storage CLI. What is the reason for this decision?* > > > For the storage tool, we are setting all the features for the cluster. By > default, all are set. For the upgrade tool, the default is to set one > feature. In the storage tool, it is natural for the --release-version to > set the remaining features that --features didn't cover since otherwise we > would need to set them all > > If we use the flag. In the feature upgrade case, it is less necessary for > all the features to be set at once and the tool can be run multiple times. > I'm not opposed to allowing both flags, but it is less necessary in my > opinion. > > > *3. KIP-853 deprecates --metadata in the kafka-features and makes it an* > > *alias for --release-version. In KIP-1022, what happens if the user* > > *specified both --metadata and --feature?* > > > See my note above (Jun's comment 12) about deprecating old commands. I > would think as the KIP stands now, we would not accept both commands. > > > *4. I would suggest keeping this* > > *consistent with kafka-features. It would avoid having to implement one* > > *more parser in Kafka.* > > > I sort of already implemented it as such, so I don't think it is too > tricky. I'm not sure of an alternative. Kafka features currently only > supports one feature at a time. > I would like to support more than one for the storage tool. Do you have > another suggestion for multiple features in the storage tool? > > > *5. As currently described, trial and error seem to be the* > > *only mechanism. Should the Kafka documentation describe these* > > *dependencies? Is that good enough?* > > > The plan so far is documentation. The idea is that this is an advanced > feature, so I think it is reasonable to ask folks use documentation > > > *6. Did you mean that 3.8-IV4 would map to TV2? If* > > *not, 3.8-IV3 would map to two different TV values.* > > > It was a typo. Each MV maps to a single other feature version. > > > *7. For --release-version in "kafka-features upgrade", does* > > *--release-version mean that all of the feature versions will either* > > *upgrade or stay the same? Meaning that downgrades will be rejected by* > > *the Kafka controller. How about when --release-version is used with* > > *"kafka-features downgrade"?* > > > The idea I had was that the cluster will check if all the versions are > supported. If any version is not supported the tool will throw an error. We > can also issue a warning if the given command (if supported by the cluster) > will downgrade a feature. One option is also to require a yes/no prompt or > flag to allow downgrades. The downgrade tool would allow the same. > Alternatively we could just fail the command if a feature is downgraded on > upgrade command or vice versa. I don't have a strong preference. > > > Thanks again for the discussion, > Justine > > On Thu, Feb 29, 2024 at 10:44 AM José Armando García Sancio > <jsan...@confluent.io.invalid> wrote: > > > Hi Justine and Jun, > > > > Thanks for the KIP Justine. See my comments below. > > > > On Wed, Feb 28, 2024 at 3:09 PM Jun Rao <j...@confluent.io.invalid> > wrote: > > > 13. KIP-853 also extends the tools to support a new feature > > kraft.version. > > > It would be useful to have alignment between that KIP and this one. > > > > I agree. I took a look at this KIP and these are the differences that > > I can spot. > > > > 1. KIP-853 uses --feature for kafka-storage instead of --features. > > This is consistent with the use of --feature in the "kafka-feature.sh > > upgrade" command. > > > > 2. I find it a bit inconsistent that --feature and --release-version > > are mutually exclusive in the kafka-feature CLI but not in the > > kafka-storage CLI. What is the reason for this decision? > > > > 3. KIP-853 deprecates --metadata in the kafka-features and makes it an > > alias for --release-version. In KIP-1022, what happens if the user > > specified both --metadata and --feature? > > > > 4. There is an example: "kafka-storage format --features > > metadata.version=16,transaction.version=2,group.coordinator.version=1". > > This is different from the --feature flag in kafka-features which is > > an append flag. Meaning that multiple features are specified as > > "--feature metadata.version=16 --feature transaction.version=2 > > --feature group.coordinator.version=1". I would suggest keeping this > > consistent with kafka-features. It would avoid having to implement one > > more parser in Kafka. > > > > 5. In the section "A Note on Feature Interdependence", you mention "an > > error should be thrown if trying to format with X version 13 and Y > > version 12". How would the user discover (or describe) these > > dependencies? As currently described, trial and error seem to be the > > only mechanism. Should the Kafka documentation describe these > > dependencies? Is that good enough? > > > > 6. In "3.8-IV2 that could map to TV1, 3.8-IV3 could also be TV1, and > > 3.8-IV3 could be TV2" did you mean that 3.8-IV4 would map to TV2? If > > not, 3.8-IV3 would map to two different TV values. > > > > 7. For --release-version in "kafka-features upgrade", does > > --release-version mean that all of the feature versions will either > > upgrade or stay the same? Meaning that downgrades will be rejected by > > the Kafka controller. How about when --release-version is used with > > "kafka-features downgrade"? > > > > -José > > >
