majialoong commented on code in PR #21627:
URL: https://github.com/apache/kafka/pull/21627#discussion_r2924397085


##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupCoordinatorConfig.java:
##########


Review Comment:
   One possible approach could be adding a new method 
`extractDynamicGroupConfigMap()` in `GroupCoordinatorConfig`.                   
                 
                                                                                
                                                                         
   The existing `extractGroupConfigMap()` only includes static configs (e.g., 
`session.timeout.ms`), which is correct for `GroupConfigManager` defaults. 
However, `validate()` also needs dynamic configs (those in 
`RECONFIGURABLE_CONFIGS`, e.g., `assignment.interval.ms`) to validate against 
the broker's current values.
                                                                                
                                                                         
   The new method could return current broker values for dynamic group configs, 
and `validate()` would merge both:
   
   ```java
   
combinedConfigs.putAll(groupCoordinatorConfig.extractGroupConfigMap(shareGroupConfig));
   
combinedConfigs.putAll(groupCoordinatorConfig.extractDynamicGroupConfigMap());
   combinedConfigs.putAll(props);
   ```
   
   This way, validation would use the broker's current dynamic values while 
keeping GroupConfigManager behavior unchanged.
   
   What do you think? Happy to discuss other approaches as well.



-- 
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]

Reply via email to