Hi, Justine, Thanks for the updated KIP.
Perhaps it's better to name the new config unstable.feature.versions.enable since there could be multiple unstable versions. Other than that, the KIP looks good to me. Jun On Thu, Apr 11, 2024 at 9:06 AM Justine Olshan <jols...@confluent.io.invalid> wrote: > The original config was never actually approved in any KIP. But we can say > it is deprecated. > I can change the config name. > > Justine > > On Thu, Apr 11, 2024 at 8:52 AM Jun Rao <j...@confluent.io.invalid> wrote: > > > Hi, Justine, > > > > Thanks for the updated KIP. > > > > Would unstable.feature.version.enable be a clearer name? Also, should we > > remove/deprecate unstable.metadata.versions.enable in this KIP? > > > > Jun > > > > On Tue, Apr 9, 2024 at 9:09 AM Justine Olshan > <jols...@confluent.io.invalid > > > > > wrote: > > > > > Hi Jun, > > > > > > Makes sense to me. It seems like KIP-1014 has been inactive recently. I > > can > > > update my KIP and mention this change on that discussion thread. > > > > > > Justine > > > > > > On Tue, Apr 9, 2024 at 9:01 AM Jun Rao <j...@confluent.io.invalid> > wrote: > > > > > > > Hi, Justine, > > > > > > > > A single config makes sense to me too. We just need to reach > consensus > > > with > > > > KIP-1014. > > > > > > > > Thanks, > > > > > > > > Jun > > > > > > > > On Mon, Apr 8, 2024 at 5:06 PM Justine Olshan > > > <jols...@confluent.io.invalid > > > > > > > > > wrote: > > > > > > > > > Hey Jun, > > > > > > > > > > That's a good question. I think maybe for simplicity, we can have a > > > > single > > > > > config? > > > > > If that makes sense, I will update the KIP. > > > > > > > > > > Justine > > > > > > > > > > On Mon, Apr 8, 2024 at 3:20 PM Jun Rao <j...@confluent.io.invalid> > > > wrote: > > > > > > > > > > > Hi, Justine, > > > > > > > > > > > > Thanks for the updated KIP. > > > > > > > > > > > > One more question related to KIP-1014. It introduced a new > > > > > > config unstable.metadata.versions.enable. Does each new feature > > need > > > to > > > > > > have a corresponding config to enable the testing of unstable > > > features > > > > or > > > > > > should we have a generic config enabling the testing of all > > unstable > > > > > > features? > > > > > > > > > > > > Jun > > > > > > > > > > > > On Thu, Apr 4, 2024 at 8:24 PM Justine Olshan > > > > > <jols...@confluent.io.invalid > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > I'm hoping this covers the majority of comments. I will go > ahead > > > and > > > > > open > > > > > > > the vote in the next day or so. > > > > > > > > > > > > > > Thanks, > > > > > > > Justine > > > > > > > > > > > > > > On Wed, Apr 3, 2024 at 3:31 PM Justine Olshan < > > > jols...@confluent.io> > > > > > > > wrote: > > > > > > > > > > > > > > > Find and replace has failed me :( > > > > > > > > > > > > > > > > Group version seems a little vague, but we can update it. > > > Hopefully > > > > > > find > > > > > > > > and replace won't fail me again, otherwise I will get another > > > email > > > > > on > > > > > > > this. > > > > > > > > > > > > > > > > Justine > > > > > > > > > > > > > > > > On Wed, Apr 3, 2024 at 12:15 PM David Jacot > > > > > > <dja...@confluent.io.invalid > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > >> Thanks, Justine. > > > > > > > >> > > > > > > > >> * Should we also use `group.version` (GV) as I suggested in > my > > > > > > previous > > > > > > > >> message in order to be consistent? > > > > > > > >> * Should we add both names to the `Public Interfaces` > section? > > > > > > > >> * There is still at least one usage of > > > > > `transaction.protocol.verison` > > > > > > in > > > > > > > >> the KIP too. > > > > > > > >> > > > > > > > >> Best, > > > > > > > >> David > > > > > > > >> > > > > > > > >> On Wed, Apr 3, 2024 at 6:29 PM Justine Olshan > > > > > > > >> <jols...@confluent.io.invalid> > > > > > > > >> wrote: > > > > > > > >> > > > > > > > >> > I had missed the David's message yesterday about the > naming > > > for > > > > > > > >> transaction > > > > > > > >> > version vs transaction protocol version. > > > > > > > >> > > > > > > > > >> > After some offline discussion with Jun, Artem, and David, > we > > > > > agreed > > > > > > > that > > > > > > > >> > transaction version is simpler and conveys more than just > > > > protocol > > > > > > > >> changes > > > > > > > >> > (flexible records for example) > > > > > > > >> > > > > > > > > >> > I will update the KIP as well as KIP-890 > > > > > > > >> > > > > > > > > >> > Thanks, > > > > > > > >> > Justine > > > > > > > >> > > > > > > > > >> > On Tue, Apr 2, 2024 at 2:50 PM Justine Olshan < > > > > > jols...@confluent.io > > > > > > > > > > > > > > >> > wrote: > > > > > > > >> > > > > > > > > >> > > Updated! > > > > > > > >> > > > > > > > > > >> > > Justine > > > > > > > >> > > > > > > > > > >> > > On Tue, Apr 2, 2024 at 2:40 PM Jun Rao > > > > <j...@confluent.io.invalid > > > > > > > > > > > > > >> wrote: > > > > > > > >> > > > > > > > > > >> > >> Hi, Justine, > > > > > > > >> > >> > > > > > > > >> > >> Thanks for the reply. > > > > > > > >> > >> > > > > > > > >> > >> 21. Sounds good. It would be useful to document that. > > > > > > > >> > >> > > > > > > > >> > >> 22. Should we add the IV in "metadata.version=17 has no > > > > > > > dependencies" > > > > > > > >> > too? > > > > > > > >> > >> > > > > > > > >> > >> Jun > > > > > > > >> > >> > > > > > > > >> > >> > > > > > > > >> > >> On Tue, Apr 2, 2024 at 11:31 AM Justine Olshan > > > > > > > >> > >> <jols...@confluent.io.invalid> > > > > > > > >> > >> wrote: > > > > > > > >> > >> > > > > > > > >> > >> > Jun, > > > > > > > >> > >> > > > > > > > > >> > >> > 21. Next producer ID field doesn't need to be > populated > > > for > > > > > TV > > > > > > 1. > > > > > > > >> We > > > > > > > >> > >> don't > > > > > > > >> > >> > have the same need to retain this since it is written > > > > > directly > > > > > > to > > > > > > > >> the > > > > > > > >> > >> > transaction log in InitProducerId. It is only needed > > for > > > > > > KIP-890 > > > > > > > >> part > > > > > > > >> > 2 > > > > > > > >> > >> / > > > > > > > >> > >> > TV 2. > > > > > > > >> > >> > > > > > > > > >> > >> > 22. We can do that. > > > > > > > >> > >> > > > > > > > > >> > >> > Justine > > > > > > > >> > >> > > > > > > > > >> > >> > On Tue, Apr 2, 2024 at 10:41 AM Jun Rao > > > > > > <j...@confluent.io.invalid > > > > > > > > > > > > > > > >> > >> wrote: > > > > > > > >> > >> > > > > > > > > >> > >> > > Hi, Justine, > > > > > > > >> > >> > > > > > > > > > >> > >> > > Thanks for the reply. > > > > > > > >> > >> > > > > > > > > > >> > >> > > 21. What about the new NextProducerId field? Will > > that > > > be > > > > > > > >> populated > > > > > > > >> > >> with > > > > > > > >> > >> > TV > > > > > > > >> > >> > > 1? > > > > > > > >> > >> > > > > > > > > > >> > >> > > 22. In the dependencies output, should we show both > > IV > > > > and > > > > > > > level > > > > > > > >> for > > > > > > > >> > >> > > metadata.version too? > > > > > > > >> > >> > > > > > > > > > >> > >> > > Jun > > > > > > > >> > >> > > > > > > > > > >> > >> > > On Mon, Apr 1, 2024 at 4:43 PM Justine Olshan > > > > > > > >> > >> > <jols...@confluent.io.invalid > > > > > > > >> > >> > > > > > > > > > > >> > >> > > wrote: > > > > > > > >> > >> > > > > > > > > > >> > >> > > > Hi Jun, > > > > > > > >> > >> > > > > > > > > > > >> > >> > > > 20. I can update the KIP. > > > > > > > >> > >> > > > > > > > > > > >> > >> > > > 21. This is used to complete some of the work > with > > > > > KIP-360. > > > > > > > (We > > > > > > > >> > use > > > > > > > >> > >> > > > previous producer ID there, but never persisted > it > > > > which > > > > > > was > > > > > > > in > > > > > > > >> > the > > > > > > > >> > >> KIP > > > > > > > >> > >> > > > > > > > > > > >> > >> > > > > > > > > > >> > >> > > > > > > > > >> > >> > > > > > > > >> > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=89068820 > > > > > > > >> > >> ) > > > > > > > >> > >> > > > The KIP also mentions including previous epoch > but > > we > > > > > > > >> explained in > > > > > > > >> > >> this > > > > > > > >> > >> > > KIP > > > > > > > >> > >> > > > how we can figure this out. > > > > > > > >> > >> > > > > > > > > > > >> > >> > > > Justine > > > > > > > >> > >> > > > > > > > > > > >> > >> > > > > > > > > > > >> > >> > > > > > > > > > > >> > >> > > > On Mon, Apr 1, 2024 at 3:56 PM Jun Rao > > > > > > > >> <j...@confluent.io.invalid> > > > > > > > >> > >> > wrote: > > > > > > > >> > >> > > > > > > > > > > >> > >> > > > > Hi, Justine, > > > > > > > >> > >> > > > > > > > > > > > >> > >> > > > > Thanks for the updated KIP. A couple of more > > > > comments. > > > > > > > >> > >> > > > > > > > > > > > >> > >> > > > > 20. Could we show the output of > version-mapping? > > > > > > > >> > >> > > > > > > > > > > > >> > >> > > > > 21. "Transaction version 1 will include the > > > flexible > > > > > > fields > > > > > > > >> in > > > > > > > >> > the > > > > > > > >> > >> > > > > transaction state log, and transaction version > 2 > > > will > > > > > > > include > > > > > > > >> > the > > > > > > > >> > >> > > changes > > > > > > > >> > >> > > > > to the transactional protocol as described by > > > KIP-890 > > > > > > > (epoch > > > > > > > >> > bumps > > > > > > > >> > >> > and > > > > > > > >> > >> > > > > implicit add partitions.)" > > > > > > > >> > >> > > > > So TV 1 enables the writing of new tagged > > fields > > > > like > > > > > > > >> > >> > PrevProducerId? > > > > > > > >> > >> > > > But > > > > > > > >> > >> > > > > those fields are only usable after the epoch > > bump, > > > > > right? > > > > > > > >> What > > > > > > > >> > >> > > > > functionality does TV 1 achieve? > > > > > > > >> > >> > > > > > > > > > > > >> > >> > > > > Jun > > > > > > > >> > >> > > > > > > > > > > > >> > >> > > > > > > > > > > > >> > >> > > > > On Mon, Apr 1, 2024 at 2:06 PM Justine Olshan > > > > > > > >> > >> > > > <jols...@confluent.io.invalid > > > > > > > >> > >> > > > > > > > > > > > > >> > >> > > > > wrote: > > > > > > > >> > >> > > > > > > > > > > > >> > >> > > > > > 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é > > > > > > > >> > >> > > > > > >> > >>>>>> > > > > > > > >> > >> > > > > > >> > >>>>> > > > > > > > >> > >> > > > > > >> > >>>> > > > > > > > >> > >> > > > > > >> > >>> > > > > > > > >> > >> > > > > > >> > >> > > > > > > > >> > >> > > > > > >> > > > > > > > > >> > >> > > > > > >> > > > > > > > > >> > >> > > > > > >> > > > > > > > >> > >> > > > > > > > > > > > > > >> > >> > > > > > > > > > > > > >> > >> > > > > > > > > > > > >> > >> > > > > > > > > > > >> > >> > > > > > > > > > >> > >> > > > > > > > > >> > >> > > > > > > > >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
