Updated. :) Thanks for the reviews Justine
On Thu, Apr 11, 2024 at 11:01 AM Jun Rao <j...@confluent.io.invalid> wrote: > 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é > > > > > > > > >> > >> > > > > > >> > >>>>>> > > > > > > > > >> > >> > > > > > >> > >>>>> > > > > > > > > >> > >> > > > > > >> > >>>> > > > > > > > > >> > >> > > > > > >> > >>> > > > > > > > > >> > >> > > > > > >> > >> > > > > > > > > >> > >> > > > > > >> > > > > > > > > > >> > >> > > > > > >> > > > > > > > > > >> > >> > > > > > >> > > > > > > > > >> > >> > > > > > > > > > > > > > > >> > >> > > > > > > > > > > > > > >> > >> > > > > > > > > > > > > >> > >> > > > > > > > > > > > >> > >> > > > > > > > > > > >> > >> > > > > > > > > > >> > >> > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
