lh0156 commented on code in PR #22935:
URL: https://github.com/apache/kafka/pull/22935#discussion_r3652899088
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupConfig.java:
##########
@@ -489,6 +492,86 @@ public static void validate(
);
}
+ /**
+ * Check a group configuration after an alter operation.
+ *
+ * <p>Only values changed by the operation are validated against the
current broker-level
+ * bounds. Values that were already stored may be outside those bounds
after a broker
+ * configuration change, but they must not prevent an unrelated group
configuration from
+ * being updated. Cross-field validation still considers the effective
value of every
+ * involved setting.</p>
+ *
+ * @param newGroupConfig The complete group config after the alter
operation.
+ * @param existingGroupConfig The complete group config before the
alter operation.
+ * @param groupCoordinatorConfig The group coordinator config.
+ * @param shareGroupConfig The share group config.
+ */
+ public static void validate(
+ Map<String, String> newGroupConfig,
+ Map<String, String> existingGroupConfig,
+ GroupCoordinatorConfig groupCoordinatorConfig,
+ ShareGroupConfig shareGroupConfig
+ ) {
Review Comment:
Addressed in 15e4b42. The alter-validation path now evaluates against the
current broker bounds, overlays only the values changed by the request, and
passes the resulting effective config through the existing path. This removes
the duplicated validation logic while preserving cross-field validation.
@squah-confluent, could you take another look when you have a chance?
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupConfig.java:
##########
@@ -489,6 +492,86 @@ public static void validate(
);
}
+ /**
+ * Check a group configuration after an alter operation.
+ *
+ * <p>Only values changed by the operation are validated against the
current broker-level
+ * bounds. Values that were already stored may be outside those bounds
after a broker
+ * configuration change, but they must not prevent an unrelated group
configuration from
+ * being updated. Cross-field validation still considers the effective
value of every
+ * involved setting.</p>
+ *
+ * @param newGroupConfig The complete group config after the alter
operation.
+ * @param existingGroupConfig The complete group config before the
alter operation.
+ * @param groupCoordinatorConfig The group coordinator config.
+ * @param shareGroupConfig The share group config.
+ */
+ public static void validate(
+ Map<String, String> newGroupConfig,
+ Map<String, String> existingGroupConfig,
+ GroupCoordinatorConfig groupCoordinatorConfig,
+ ShareGroupConfig shareGroupConfig
+ ) {
Review Comment:
Addressed in 15e4b42. The alter-validation path now evaluates
`oldGroupConfig` against the current broker bounds, overlays only the values
changed by the request, and passes the resulting effective config through the
existing `GroupConfig.validate(...)` path. This removes the duplicated
validation logic while preserving cross-field validation. @squah-confluent,
could you take another look when you have a chance?
--
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]
