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