Hi Justine, Thank you for the KIP. I think the KIP is pretty clear and makes sense to me. Maybe it would be good to give a little more detail on the implementation of feature mapping and how the tool would validate the feature combinations. For example, I'd expect that
bin/kafka-storage.sh format --release-version 3.6-IVI --feature transaction.version=2 would give an error because the new transaction protocol is not supported in 3.6. Also, we may decide that bin/kafka-storage.sh format --release-version 5.0-IV0 --feature transaction.version=0 would be an unsupported combination as it'll have been a while since the new transaction protocol has been the default and it would be too risky to enable this combination as it may not be tested any more. As for the new names, I'm thinking of the "transaction feature version" more like a "transaction protocol version" -- from the user perspective we don't really add new functionality in KIP-890, we're changing the protocol to be more robust (and potentially faster). -Artem On Wed, Feb 28, 2024 at 10:08 AM Justine Olshan <jols...@confluent.io.invalid> wrote: > Hey Andrew, > > Thanks for taking a look. > > I previously didn't include 1. We do plan to use these features immediately > for KIP-890 and KIP-848. If we think it is easier to put the discussion in > those KIP discussions we can, but I fear that it will easily get lost given > the size of the KIPs. > > I named the features similar to how we named metadata version. Transaction > version would control transaction features like enabling a new transaction > record format and APIs to enable KIP-890 part 2. Likewise, the group > coordinator version would also enable the new record formats there and the > new group coordinator. I am open to new names or further discussion. > > For 2 and 3, I can provide example scripts that show the usage. I am open > to adding --latest-stable as well. > > Justine > > On Tue, Feb 27, 2024 at 4:59 AM Andrew Schofield < > andrew_schofield_j...@outlook.com> wrote: > > > Hi Justine, > > Thanks for the KIP. This area of Kafka is complicated and making it > easier > > is good. > > > > When I use the `kafka-features.sh` tool to describe the features on my > > cluster, I find that there’s a > > single feature called “metadata.version”. I think this KIP does a handful > > of things: > > > > 1) It introduces the idea of two new features, TV and GCV, without giving > > them concrete names or > > describing their behaviour. > > 2) It introduces a new flag on the storage tool to enable advanced users > > to control individual features > > when they format storage for a new broker. > > 3) It introduces a new flag on the features tool to enable a set of > latest > > stable features for a given > > version to be enabled all together. > > > > I think that (1) probably shouldn’t be in this KIP unless there are > > concrete details. Maybe this KIP is enabling > > the operator experience when we introduce TV and GCV in other KIPs. I > > don’t believe the plan is to enable > > the new group coordinator with a feature, and it seems unnecessary to me. > > I think it’s more compelling for TV > > given the changes in transactions. > > > > For (2) and (3), it would be helpful to explicit about the syntax for the > > enhancements to the tool. I think > > that for the features tool, `--release-version` is an optional parameter > > which requires a RELEASE_VERSION > > argument. I wonder whether it would be helpful to have `--latest-stable` > > as an option too. > > > > Thanks, > > Andrew > > > > > On 26 Feb 2024, at 21:26, Justine Olshan <jols...@confluent.io.INVALID > > > > wrote: > > > > > > Hello folks, > > > > > > I'm proposing a KIP that allows for setting and upgrading new features > > > (other than metadata version) via the kafka storage format and feature > > > tools. This KIP extends on the feature versioning changes introduced by > > > KIP-584 by allowing for the features to be set and upgraded. > > > > > > Please take a look: > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-1023%3A+Formatting+and+Updating+Features > > > > > > Thanks, > > > > > > Justine > > > > >
