Hi Justine, Thanks for the update. See my comments below.
On Mon, Mar 25, 2024 at 2:51 PM Justine Olshan <jols...@confluent.io.invalid> wrote: > I've updated the KIP with the changes I mentioned earlier. I have not yet > removed the --feature-version flag from the upgrade tool. What's the "--feature-version" flag? This is the first time I see it mentioned and I don't see it in the KIP. Did you mean "--release-version"? > Please take a look at the API to get the versions for a given > metadata version. Right now I'm using ApiVersions request and specifying a > metadata version. The supported versions are then supplied in the > ApiVersions response. > There were tradeoffs with this approach. It is a pretty minimal change, but > reusing the API means that it could be confusing (ie, the ApiKeys will be > supplied in the response but not needed.) I considered making a whole new > API, but that didn't seem necessary for this use. I agree that this is extremely confusing and we shouldn't overload the ApiVersions RPC to return this information. The KIP doesn't mention how it is going to use this API. Do you need to update the Admin client to include this information? Having said this, as you mentioned in the KIP the kafka-storage tool needs this information and that tool cannot assume that there is a running server it can query (send an RPC). Can the kafka-features use the same mechanism used by kafka-storage without calling into a broker? re: "It will work just like the storage tool and upgrade all the features to a version" Does this mean that --release-version cannot be used with "kafka-features downgrade"? re: Consistency with KIP-853 Jun and I have been having a similar conversation in the discussion thread for KIP-853. From what I can tell both changes are compatible. Do you mind taking a look at these two sections and confirming that they don't contradict your KIP? 1. https://cwiki.apache.org/confluence/display/KAFKA/KIP-853%3A+KRaft+Controller+Membership+Changes#KIP853:KRaftControllerMembershipChanges-kafka-storage 2. https://cwiki.apache.org/confluence/display/KAFKA/KIP-853%3A+KRaft+Controller+Membership+Changes#KIP853:KRaftControllerMembershipChanges-kafka-features re: nit "For MVs that existed before these features, we map the new features to version 0 (no feature enabled)." To me version 0 doesn't necessarily mean that the feature is not enabled. For example, for kraft.version, version 0 means the protocol prior to KIP-853. Version 0 is the currently implemented version of the KRaft protocol. Thanks, -- -José
