[
https://issues.apache.org/jira/browse/KAFKA-19028?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18108155#comment-18108155
]
Shantanu commented on KAFKA-19028:
----------------------------------
I looked at current trunk ({{ConfigurationControlManager}}) and confirmed the
policy map with unit tests that capture
{{AlterConfigPolicy.RequestMetadata.configs()}} on incremental DELETE/SUBTRACT.
{{AlterConfigPolicy}} runs on the controller. The map passed to the policy is
built from the {{ConfigRecord}}s generated for the request, not from the Admin
ops directly. DELETE sets {{newValue = null}}. A record is generated when
{{currentValue}} differs from {{newValue}}, or when the resource type is BROKER
(brokers reload dynamic config).
Unit tests on trunk:
|| Scenario || ConfigRecord generated? || Policy map ||
| TOPIC DELETE of a key that is currently stored | yes | key present, value
null |
| TOPIC DELETE of a key that is not stored | no | empty map (key absent) |
| BROKER DELETE of a key that is currently stored | yes | key present, value
null |
| BROKER DELETE of a key that is not stored | yes | key present, value null |
| BROKER SUBTRACT on an unset LIST key whose schema default is "1", subtracting
"1" | yes | key present, value empty string (computed result, not the default) |
| TOPIC DELETE of a stored BOOLEAN whose schema default is "true" | yes | key
present, value null (not "true") |
Example: {{DELETE retention.ms}} on a topic that already has a topic-level
override → policy sees {{retention.ms=null}}. The same DELETE on a topic with
no override → policy sees an empty map. The same DELETE on a broker resource
always sees {{retention.ms=null}}.
Approaches:
*A. Document current behavior, no code change*
Keep today’s map. Javadoc on {{RequestMetadata.configs()}} would state that it
is the map of generated records: SET/APPEND/SUBTRACT → new string; DELETE →
null if a record was produced; omitted if the op was a no-op (typical for a
topic key that was never overridden).
Example: {{DELETE retention.ms}} on a topic already on the default → policy
sees an empty map. Same op on a broker → {{retention.ms=null}}.
Why we would not pick this as the only outcome: TOPIC and BROKER DELETE still
differ when the topic key is not stored. A policy cannot apply the same DELETE
rule to both without checking resource type and whether an override exists.
*B. Always include an explicit DELETE as key=null*
Build the policy map from the requested ops. If the client sent DELETE, the map
always has the key with null, even when no {{ConfigRecord}} is generated.
Example: {{DELETE retention.ms}} on topic {{orders}} with an override →
{{retention.ms=null}} (same as today). Without an override →
{{retention.ms=null}} instead of an empty map. Broker DELETE stays
{{retention.ms=null}}.
This matches “configs in the request.” Policies can treat {{containsKey}} +
null as DELETE for every resource type.
Issue: plugins that currently see an empty map on a no-op topic DELETE would
start seeing key=null.
*C. Put the schema default on DELETE*
On DELETE, put the default string when the key has one (for example
{{retention.ms}} → the schema default). Keys with no default would still be
null.
Example: topic has {{retention.ms=86400000}}, then DELETE → {{retention.ms}}
set to the schema default instead of null.
Why we would not pick this: SET-to-default and DELETE would look the same.
SUBTRACT already passes the computed list, not the default, so this is not
consistent with SUBTRACT. A topic DELETE that writes no record would still show
a default even though metadata did not change.
*D. Pass the full post-apply snapshot*
Pass {{allConfigs}} after applying the ops, not only the keys in this request.
DELETE would remove the key from that snapshot.
Example: topic currently {{retention.ms=86400000}}, {{cleanup.policy=compact}},
DELETE {{retention.ms}} → {{cleanup.policy=compact}} (retention absent).
Why we would not pick this: DELETE becomes less visible. Existing tests expect
unrelated existing configs not to be passed. Broker DELETE would no longer
appear as key=null.
*E. Add structured ops on RequestMetadata*
Keep or adjust {{configs()}} and add SET/DELETE/APPEND/SUBTRACT on
{{RequestMetadata}} so the policy sees the op type.
Example: {{DELETE retention.ms}} → op=DELETE, name={{retention.ms}} for both
topic and broker.
Issue: {{AlterConfigPolicy}} is a public plugin interface. New fields would be
a public-API change and would need a KIP.
I lean toward *B* plus javadoc on {{RequestMetadata.configs()}}, unless the
record-based map is considered intentional.
> AlterConfigPolicy with OpType.DELETE : inconsistent behavior between TOPIC
> and BROKER resource
> ----------------------------------------------------------------------------------------------
>
> Key: KAFKA-19028
> URL: https://issues.apache.org/jira/browse/KAFKA-19028
> Project: Kafka
> Issue Type: Bug
> Reporter: Edoardo Comar
> Priority: Major
>
> During an IncrementalAlterConfig that DELETES a config entry, the configs map
> that an AlterConfigPolicy will inspect appears to be inconsistent between the
> deletion of a broker property and the deletion of a topic property :
> when deleting a Topic property, the config entry is missing from the Map
> passed to the policy
> when deleting a Broker property, the config entry is present in the Map
> passed to the policy but the associated value is null.
> For consistency with the case of an OpType.SUBTRACT (which limited to a
> config entry of type List) the Map could instead contain the default value of
> the deleted property, if it exists.
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
