I have also updated the KIP to mention the feature tool's --metadata flag will be deprecated. It will still work for users as they learn the new flag, but a warning indicating the alternatives will be shown.
Justine On Thu, Mar 28, 2024 at 11:03 AM Justine Olshan <jols...@confluent.io> wrote: > Hi Jun, > > For both transaction state and group coordinator state, there are only > version 0 records. > KIP-915 introduced flexible versions, but it was never put to use. MV has > never gated these. This KIP will do that. I can include this context in the > KIP. > > I'm happy to modify his 1 and 2 to 0 and 1. > > Justine > > On Thu, Mar 28, 2024 at 10:57 AM Jun Rao <j...@confluent.io.invalid> wrote: > >> Hi, David, >> >> Thanks for the reply. >> >> Historically, the format of all records were controlled by MV. Now, >> records >> in _offset_commit will be controlled by `group.coordinator.version`, is >> that right? It would be useful to document that. >> >> Also, we should align on the version numbering. "kafka-feature disable" >> says "Disable one or more feature flags. This is the same as downgrading >> the version to zero". So, in the `group.coordinator.version' case, we >> probably should use version 0 for the old consumer protocol. >> >> Jun >> >> On Thu, Mar 28, 2024 at 2:13 AM Andrew Schofield < >> andrew_schofield_j...@outlook.com> wrote: >> >> > Hi David, >> > I agree that we should use the same mechanism to gate KIP-932 once that >> > feature reaches production readiness. The precise details of the values >> > will >> > depend upon the current state of all these flags when that release >> comes. >> > >> > Thanks, >> > Andrew >> > >> > > On 28 Mar 2024, at 07:11, David Jacot <dja...@confluent.io.INVALID> >> > wrote: >> > > >> > > Hi, Jun, Justine, >> > > >> > > Regarding `group.coordinator.version`, the idea is to use it to gate >> > > records and APIs of the group coordinator. The first use case will be >> > > KIP-848. We will use version 2 of the flag to gate all the new records >> > and >> > > the new ConsumerGroupHeartbeat/Describe APIs present in AK 3.8. So >> > version >> > > 1 will be the only the old protocol and version 2 will be the >> currently >> > > implemented new protocol. I don't think that we have any dependency on >> > the >> > > metadata version at the moment. The changes are orthogonal. I think >> that >> > we >> > > could mention KIP-848 as the first usage of this flag in the KIP. I >> will >> > > also update KIP-848 to include it when this KIP is accepted. Another >> use >> > > case is the Queues KIP. I think that we should also use this new flag >> to >> > > gate it. >> > > >> > > Best, >> > > David >> > > >> > > On Thu, Mar 28, 2024 at 1:14 AM Jun Rao <j...@confluent.io.invalid> >> > wrote: >> > > >> > >> Hi, Justine, >> > >> >> > >> Thanks for the reply. >> > >> >> > >> So, "dependencies" and "version-mapping" will be added to both >> > >> kafka-feature and kafka-storage? Could we document that in the tool >> > format >> > >> section? >> > >> >> > >> Jun >> > >> >> > >> On Wed, Mar 27, 2024 at 4:01 PM Justine Olshan >> > >> <jols...@confluent.io.invalid> >> > >> wrote: >> > >> >> > >>> Ok. I can remove the info from the describe output. >> > >>> >> > >>> Dependencies is needed for the storage tool because we want to make >> > sure >> > >>> the desired versions we are setting will be valid. Version mapping >> > should >> > >>> be for both tools since we have --release-version for both tools. >> > >>> >> > >>> I was considering changing the IV strings, but I wasn't sure if >> there >> > >> would >> > >>> be some disagreement with the decision. Not sure if that breaks >> > >>> compatibility etc. Happy to hear everyone's thoughts. >> > >>> >> > >>> Justine >> > >>> >> > >>> On Wed, Mar 27, 2024 at 3:36 PM Jun Rao <j...@confluent.io.invalid> >> > >> wrote: >> > >>> >> > >>>> Hi, Justine, >> > >>>> >> > >>>> Thanks for the reply. >> > >>>> >> > >>>> Having "kafka-feature dependencies" seems enough to me. We don't >> need >> > >> to >> > >>>> include the dependencies in the output of "kafka-feature describe". >> > >>>> >> > >>>> We only support "dependencies" in kafka-feature, not >> kafka-storage. We >> > >>>> probably should do the same for "version-mapping". >> > >>>> >> > >>>> bin/kafka-features.sh downgrade --feature metadata.version=16 >> > >>>> --transaction.protocol.version=2 >> > >>>> We need to add the --feature flag for the second feature, right? >> > >>>> >> > >>>> In "kafka-features.sh describe", we only show the IV string for >> > >>>> metadata.version. Should we also show the level number? >> > >>>> >> > >>>> Thanks, >> > >>>> >> > >>>> Jun >> > >>>> >> > >>>> On Wed, Mar 27, 2024 at 1:52 PM Justine Olshan >> > >>>> <jols...@confluent.io.invalid> >> > >>>> wrote: >> > >>>> >> > >>>>> I had already included this example >> > >>>>> bin/kafka-features.sh downgrade --feature metadata.version=16 >> > >>>>> --transaction.protocol.version=2 // Throws error if metadata >> version >> > >>> is < >> > >>>>> 16, and this would be an upgrade >> > >>>>> But I have updated the KIP to explicitly say the text you >> mentioned. >> > >>>>> >> > >>>>> Justine >> > >>>>> >> > >>>>> On Wed, Mar 27, 2024 at 1:41 PM José Armando García Sancio >> > >>>>> <jsan...@confluent.io.invalid> wrote: >> > >>>>> >> > >>>>>> Hi Justine, >> > >>>>>> >> > >>>>>> See my comment below. >> > >>>>>> >> > >>>>>> On Wed, Mar 27, 2024 at 1:31 PM Justine Olshan >> > >>>>>> <jols...@confluent.io.invalid> wrote: >> > >>>>>>> The feature command includes the upgrade or downgrade command >> > >> along >> > >>>>> with >> > >>>>>>> the --release-version flag. If some features are not moving in >> > >> the >> > >>>>>>> direction mentioned (upgrade or downgrade) the command will fail >> > >> -- >> > >>>>>> perhaps >> > >>>>>>> with an error of which features were going in the wrong >> > >> direction. >> > >>>>>> >> > >>>>>> How about updating the KIP to show and document this behavior? >> > >>>>>> >> > >>>>>> Thanks, >> > >>>>>> -- >> > >>>>>> -José >> > >>>>>> >> > >>>>> >> > >>>> >> > >>> >> > >> >> > >> > >> >
