Skip to content

Commit bd97a2a

Browse files
authored
Refactor newTopics to remove nested loops (#4026)
* Refactor newTopics to remove nested loops Replace O(n²) nested iteration with O(n) map-based lookups for filtering topics Use topicNameToMapKey to efficiently resolve duplicate topic names This improves application startup time and scalability when managing a large number of topics, while preserving the original filtering logic Resolves: #4025 * Refactor newTopics filtering with removeIf Replace manual Iterator loop with Collection.removeIf() to improve code readability and maintainability. The original implementation used a while loop with Iterator to remove entries that don't match the createOrModifyTopic predicate. While this worked, it required more boilerplate code and was less expressive. The new implementation uses the removeIf method introduced in Java 8, which directly expresses the filtering operation in a single line. This reduces method complexity and makes the code more idiomatic. * Simplify newTopics logic to use List instead of Map Refactored newTopics() to directly collect NewTopic and NewTopics beans into a single List<NewTopic>, avoiding unnecessary use of Map and bean names. TopicForRetryable instances are now removed if a regular NewTopic with the same name exists. This simplifies the logic and improves code readability. The final filtering is performed using removeIf for clarity and performance. * Refactor topic filtering in KafkaAdmin.newTopics Merge dual removeIf operations into a single filter condition. Rename normalNames to regularTopicNames for improved clarity. This change optimizes performance by reducing the number of collection iterations and improves code readability by using clearer naming. Signed-off-by: Choi Wang Gyu <[email protected]>
1 parent d4925d6 commit bd97a2a

File tree

1 file changed

+22
-37
lines changed

1 file changed

+22
-37
lines changed

spring-kafka/src/main/java/org/springframework/kafka/core/KafkaAdmin.java

Lines changed: 22 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,11 @@
2222
import java.util.Collection;
2323
import java.util.Collections;
2424
import java.util.HashMap;
25-
import java.util.Iterator;
2625
import java.util.LinkedList;
2726
import java.util.List;
2827
import java.util.Map;
29-
import java.util.Map.Entry;
3028
import java.util.Optional;
29+
import java.util.Set;
3130
import java.util.concurrent.ExecutionException;
3231
import java.util.concurrent.TimeUnit;
3332
import java.util.concurrent.TimeoutException;
@@ -323,41 +322,27 @@ private void updateClusterId(Admin adminClient) throws InterruptedException, Exe
323322
*/
324323
protected Collection<NewTopic> newTopics() {
325324
Assert.state(this.applicationContext != null, "'applicationContext' cannot be null");
326-
Map<String, NewTopic> newTopicsMap = new HashMap<>(
327-
this.applicationContext.getBeansOfType(NewTopic.class, false, false));
328-
Map<String, NewTopics> wrappers = this.applicationContext.getBeansOfType(NewTopics.class, false, false);
329-
AtomicInteger count = new AtomicInteger();
330-
wrappers.forEach((name, newTopics) -> {
331-
newTopics.getNewTopics().forEach(nt -> newTopicsMap.put(name + "#" + count.getAndIncrement(), nt));
332-
});
333-
Map<String, NewTopic> topicsForRetry = newTopicsMap.entrySet().stream()
334-
.filter(entry -> entry.getValue() instanceof TopicForRetryable)
335-
.collect(Collectors.toMap(Entry::getKey, Entry::getValue));
336-
for (Entry<String, NewTopic> entry : topicsForRetry.entrySet()) {
337-
Iterator<Entry<String, NewTopic>> iterator = newTopicsMap.entrySet().iterator();
338-
boolean remove = false;
339-
while (iterator.hasNext()) {
340-
Entry<String, NewTopic> nt = iterator.next();
341-
// if we have a NewTopic and TopicForRetry with the same name, remove the latter
342-
if (nt.getValue().name().equals(entry.getValue().name())
343-
&& !(nt.getValue() instanceof TopicForRetryable)) {
344-
345-
remove = true;
346-
break;
347-
}
348-
}
349-
if (remove) {
350-
newTopicsMap.remove(entry.getKey());
351-
}
352-
}
353-
Iterator<Entry<String, NewTopic>> iterator = newTopicsMap.entrySet().iterator();
354-
while (iterator.hasNext()) {
355-
Entry<String, NewTopic> next = iterator.next();
356-
if (!this.createOrModifyTopic.test(next.getValue())) {
357-
iterator.remove();
358-
}
359-
}
360-
return new ArrayList<>(newTopicsMap.values());
325+
326+
// Deal with List<NewTopic> directly instead of Map (no need for bean names)
327+
List<NewTopic> newTopicsList = new ArrayList<>(
328+
this.applicationContext.getBeansOfType(NewTopic.class, false, false).values());
329+
330+
// Add topics from NewTopics wrappers (no need for bean names either)
331+
this.applicationContext.getBeansOfType(NewTopics.class, false, false).values()
332+
.forEach(wrapper -> newTopicsList.addAll(wrapper.getNewTopics()));
333+
334+
// Collect regular topic names to check against TopicForRetryable
335+
Set<String> regularTopicNames = newTopicsList.stream()
336+
.filter(nt -> !(nt instanceof TopicForRetryable))
337+
.map(NewTopic::name)
338+
.collect(Collectors.toSet());
339+
340+
// Apply combined filter: remove TopicForRetryable with same name as regular topic OR topics that don't pass predicate
341+
newTopicsList.removeIf(nt ->
342+
(nt instanceof TopicForRetryable && regularTopicNames.contains(nt.name())) ||
343+
!this.createOrModifyTopic.test(nt));
344+
345+
return newTopicsList;
361346
}
362347

363348
@Override

0 commit comments

Comments
 (0)