Hi, Justine, unstable.metadata.versions.enable is an internal configuration. So, we could probably just remove it instead of depreciation. Also, it would be useful to make it clear that unstable.feature.versions.enable is an internal configuration.
Thanks, Jun On Thu, Apr 11, 2024 at 11:16 AM Justine Olshan <jols...@confluent.io.invalid> wrote: > 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é > > > > > > > > > >> > >> > > > > > >> > >>>>>> > > > > > > > > > >> > >> > > > > > >> > >>>>> > > > > > > > > > >> > >> > > > > > >> > >>>> > > > > > > > > > >> > >> > > > > > >> > >>> > > > > > > > > > >> > >> > > > > > >> > >> > > > > > > > > > >> > >> > > > > > >> > > > > > > > > > > >> > >> > > > > > >> > > > > > > > > > > >> > >> > > > > > >> > > > > > > > > > >> > >> > > > > > > > > > > > > > > > >> > >> > > > > > > > > > > > > > > >> > >> > > > > > > > > > > > > > >> > >> > > > > > > > > > > > > >> > >> > > > > > > > > > > > >> > >> > > > > > > > > > > >> > >> > > > > > > > > > >> > > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
