Hi David, Thanks again for the KIP.
David Arthur wrote: > 101. Change AllowDowngrade bool to DowngradeType int8 in > UpgradeFeatureRequest RPC. I'm wondering if we can kind of "cheat" on this > incompatible change since it's not currently in use and totally remove the > old field and leave the versions at 0+. Thoughts? We've been talking about having an automated RPC compatibility checker, and doing something like this might make that more complex. On the other hand, I suppose it could be annotated as a special case. David Arthur wrote: > The active controller should probably validate whatever value is read from > meta.properties against its own range of supported versions (statically > defined in code). If the operator sets a version unsupported by the active > controller, that sounds like a configuration error and we should shutdown. > I'm not sure what other validation we could do here without introducing > ordering dependencies (e.g., must have quorum before initializing the > version) It would be nice if the active controller could validate that a majority of the quorum could use the proposed metadata.version. The active controller should have this information, right? If we don't have recent information from a quorum of voters, we wouldn't be active. David Arthur wrote: > Hey folks, I just updated the KIP with details on proposed changes to the > kafka-features.sh tool. It includes four proposed sub-commands which will > provide the Basic and Advanced functions detailed in KIP-584. Please have a > look, thanks! I like the new sub-commands... seems very clean. Do we need delete as a command separate from downgrade? In KIP-584, we agreed to keep version 0 reserved for "no such feature flag." So downgrading to version 0 should be the same as deletion. On another note, it seems like we should spell out that metadata.version begins at 1 in KRaft clusters, and that it's always 0 in ZK-based clusters. Maybe this is obvious but it would be good to write it down here. It seems like we never got closure on the issue of simplifying feature levels. As I said earlier, I think the min/max thing is not needed for 99% of the use-cases we've talked about for feature levels. Certainly, it's not needed for metadata.version. Having it be mandatory for every feature level, whether it needs it or not, is an extra complication that I think we should get rid of, before this interface becomes set in stone. (Use-cases that need this can simply have two feature flags, X_min and X_max, as I said before.) We probably also want an RPC implemented by both brokers and controllers that will reveal the min and max supported versions for each feature level supported by the server. This is useful for diagnostics. And I suppose we should have a command line option to the features command that broadcasts it to everyone and returns the results (or lack of result, if the connection couldn't be made...) best, Colin
