andrewgrantcflt commented on PR #17928: URL: https://github.com/apache/kafka/pull/17928#issuecomment-3746422645
@chia7712 @HyunSangHan @divijvaidya does this not break being able to call IncrementalAlterConfigs for topics that already have the `message.timestamp.difference.max.ms` set? Looking at the code [here](https://github.com/apache/kafka/blob/trunk/core/src/main/scala/kafka/server/ControllerConfigurationValidator.scala#L120) I see we validate `properties`. In `LogConfig` the validation code is ``` public static void validate( Map<String, String> existingConfigs, Properties props, Map<?, ?> configuredProps, boolean isRemoteLogStorageSystemEnabled, boolean isVirtualTopicCreationEnabled ) { validateNames(props); if (configuredProps == null || configuredProps.isEmpty()) { Map<?, ?> valueMaps = CONFIG.parse(props); validateValues(valueMaps); } else { Map<Object, Object> combinedConfigs = new HashMap<>(configuredProps); combinedConfigs.putAll(props); Map<?, ?> valueMaps = CONFIG.parse(combinedConfigs); validateTopicLogConfigValues(existingConfigs, valueMaps, isRemoteLogStorageSystemEnabled, isVirtualTopicCreationEnabled); } } ``` So for `properties` we call `validateNames` which fails in the config name does not exist. It looks like `properties` gets populated with the data from `newConfigs`. Looking at the caller in https://github.com/apache/kafka/blob/trunk/metadata/src/main/java/org/apache/kafka/controller/ConfigurationControlManager.java#L325-L332 it looks like that data contains the old configs and new ones. So if a topic already had `message.timestamp.difference.max.ms` won't you get blocked altering the topic config? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
