[GitHub] [kafka] C0urante commented on pull request #13373: Kafka-14420 Use incrementalAlterConfigs API for syncing topic configurations (KIP-894)

["C0urante (via GitHub)" <gi...@apache.org>]

C0urante commented on PR #13373:
URL: https://github.com/apache/kafka/pull/13373#issuecomment-1488710656

   @tinaselenge I owe you a big apology!
   
   I think the API you had for the `ConfigPropertyFilter` interface was correct the first time around.
   
   The double-check logic is necessary because we need to distinguish between three possible cases when incrementally altering topic configs:
   
   0. A property with a default value on the source topic should be synced to the target topic (using a `SET` operation in the config alter request)
   0. A property with a default value on the source topic should use the default value on the target topic (which is accomplished via a `DELETE` operation in the config alter request)
   0. A property with a default value on the source topic should not be altered on the target topic at all
   
   If we only rely on `ConfigPropertyFilter::shouldReplicateSourceDefault`, that doesn't give us enough information to choose between those three options (since the return value of that method can only have two different values).
   
   Instead, we can and should use both methods to decide how to act (for some property that has a default value on the source cluster):
   
   0. If both `ConfigPropertyFilter::shouldReplicateConfigProperty` and `ConfigPropertyFilter::shouldReplicateSourceDefault` return `true`, then we sync the property to the target cluster using `SET`
   0. If `ConfigPropertyFilter::shouldReplicateConfigProperty` returns `true` but `ConfigPropertyFilter::shouldReplicateSourceDefault` returns `false`, then we sync the property to the target cluster using `DELETE`
   0. If `ConfigPropertyFilter::shouldReplicateConfigProperty` returns `false`, we do not sync the property at all
   
   So I've come to realize that the way you'd originally implemented this is actually the only correct way to satisfy the design outlined in the KIP.
   
   I'm very sorry for wasting your time on this wild goose chase, please accept my apologies!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org