KAFKA-18600 Cleanup NetworkClient zk related logging #18644
Conversation
| node, apiVersionsResponse.data().finalizedFeaturesEpoch(), apiVersionsResponse.data().finalizedFeatures(), | ||
| apiVersionsResponse.data().supportedFeatures(), apiVersionsResponse.data().zkMigrationReady(), nodeVersionInfo); |
There was a problem hiding this comment.
(Unrelated to this PR) @mumrah Do we have a plan to remove this field from the protocol api? We would need to ensure it's done in a compatible way, but it would be nice not to have this field in request logs and general logging.
There was a problem hiding this comment.
Since it's a tagged field and 4.0 cannot be the target of a ZK migration, we could just remove it without a version bump. However, we might consider keeping it around so that 4.0+ controllers can reject errant registrations from a Zk broker (like due to a misconfiguration).
There was a problem hiding this comment.
Interesting, would this be from a broker that is being migrated?
There was a problem hiding this comment.
Oh actually I was confusing this with the same field in the broker registration request. Looking at usages in 3.9, I'm not sure we ever used this field. I'll dig in a bit on this.
There was a problem hiding this comment.
This field (ZkMigrationReady in ApiVersionsResponse) was last used in 3.6. Starting in 3.7, we used the controller registration RPC in lieu of this field. So, it seems a bit like dead code at this point. We should probably leave the field in the RPC so we don't reuse the tagged field number, but we can set the max versions to v4 and remove the usages in the non-generated code.
There was a problem hiding this comment.
we can set the max versions to v4
Pardon me, it seems to me the max version is already set to v4 (ApiKeys.API_VERSIONS.latestVersion()). Please correct me if I misunderstand anything.
we might consider keeping it around so that 4.0+ controllers can reject errant registrations from a Zk broker (like due to a misconfiguration).
it seems that is addressed already.
remove the usages in the non-generated code.
I prefer to remove isMigratingZkBroker and IsMigratingZkBroker from non-generated code. similar to my previous comment #17293 (comment)
Jira: Cleanup NetworkClient zk related logging
Committer Checklist (excluded from commit message)