lianetm commented on code in PR #15271:
URL: https://github.com/apache/kafka/pull/15271#discussion_r1467947528
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/MembershipManagerImpl.java:
##########
@@ -1311,21 +1294,34 @@ public Map<Uuid, SortedSet<Integer>>
currentAssignment() {
return this.currentAssignment;
}
-
/**
* @return Set of topic IDs received in a target assignment that have not
been reconciled yet
- * because topic names are not in metadata. Visible for testing.
+ * because topic names are not in metadata or reconciliation hasn't
finished. Visible for testing.
*/
- Set<Uuid> topicsWaitingForMetadata() {
- return Collections.unmodifiableSet(assignmentUnresolved.keySet());
+ Set<Uuid> topicsAwaitingReconciliation() {
Review Comment:
We are slightly changing the concept of what this does, and I'm fine with it
because it's ok for how it's used in the tests, but I think the
description/name/implementation are not well aligned? This is not strictly
getting topicsAwaitingReconciliation ("not in metadata or reconciliation hasn't
finished"), ex. member owns one partitions of the topic, and another one is
being added and stuck reconciling (committing offsets, revoking, etc), that
topic wouldn't be returned here but it's indeed a
`topicsAwaitingReconciliation` (for which "reconciliation hasn't finished").
What about we just rely on the`topicPartitionsAwaitingReconciliation` you
defined below (that seems accurate to me), and here we just `return
topicPartitionsAwaitingReconciliation().keySet();`
I do know this is not causing trouble because of how it is used in the tests
at the moment, but better to get it right to avoid confusion/misuse.
--
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
