lianetm commented on code in PR #15271:
URL: https://github.com/apache/kafka/pull/15271#discussion_r1467835947
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/MembershipManagerImpl.java:
##########
@@ -1340,18 +1336,15 @@ boolean reconciliationInProgress() {
* When cluster metadata is updated, try to resolve topic names for topic
IDs received in
* assignment that hasn't been resolved yet.
* <ul>
- * <li>Try to find topic names for all unresolved assignments</li>
+ * <li>Try to find topic names for all assignments</li>
* <li>Add discovered topic names to the local topic names cache</li>
* <li>If any topics are resolved, trigger a reconciliation
process</li>
* <li>If some topics still remain unresolved, request another
metadata update</li>
* </ul>
*/
@Override
public void onUpdate(ClusterResource clusterResource) {
- resolveMetadataForUnresolvedAssignment();
- if (!assignmentReadyToReconcile.isEmpty()) {
- reconcile();
- }
+ reconcile();
Review Comment:
We should probably reconcile here only if we're in the RECONCILING state I
would say. We could receive metadata updates anytime. With the current
reconcile implementation I expect nothing bad would happen by calling this
reconcile on a metadata update when there is nothing to reconcile or are in
some other state/transition, but probably clearer/safer to reason about it with
a short-circuit here to avoid unexpected effects/logs?
--
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-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
