k-apol commented on code in PR #20063:
URL: https://github.com/apache/kafka/pull/20063#discussion_r2247716436
##########
streams/src/main/java/org/apache/kafka/streams/processor/internals/InternalTopicManager.java:
##########
@@ -461,120 +461,125 @@ public Set<String> makeReady(final Map<String,
InternalTopicConfig> topics) {
// have existed with the expected number of partitions, or some create
topic returns fatal errors.
log.debug("Starting to validate internal topics {} in partition
assignor.", topics);
- long currentWallClockMs = time.milliseconds();
+ final long currentWallClockMs = time.milliseconds();
final long deadlineMs = currentWallClockMs + retryTimeoutMs;
- Set<String> topicsNotReady = new HashSet<>(topics.keySet());
+ final Set<String> topicsNotReady = new HashSet<>(topics.keySet());
final Set<String> newlyCreatedTopics = new HashSet<>();
while (!topicsNotReady.isEmpty()) {
- final Set<String> tempUnknownTopics = new HashSet<>();
- topicsNotReady = validateTopics(topicsNotReady, topics,
tempUnknownTopics);
- newlyCreatedTopics.addAll(topicsNotReady);
-
+ final Set<NewTopic> validatedTopicObjects =
createValidatedTopicObjects(topics, topicsNotReady, newlyCreatedTopics);
Review Comment:
I thought it would be easier to reason about the current state of this
method by writing it this way. The motivation for this was that I got a bit
confused reading through the old code because they are not really 'topics'
officially until we call the admin client. It was easier for me to reason about
by being more explicit about their current state at this point in the method, I
thought other maintainers would feel the same.
But, I see your point and also feel this is better - we are still
communicating that they are not yet 'real' but with less and simpler wording.
Making an update
--
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
