ableegoldman commented on code in PR #12594:
URL: https://github.com/apache/kafka/pull/12594#discussion_r989672094
##########
streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamsMetadataState.java:
##########
@@ -389,22 +389,18 @@ private List<StreamsMetadata>
rebuildMetadataForNamedTopologies(final Map<HostIn
standbyStoresOnHost.addAll(getStoresOnHost(storeToSourceTopics,
standbyPartitionsOnHost));
}
- if (!(activeStoresOnHost.isEmpty() &&
activePartitionsOnHost.isEmpty() && standbyStoresOnHost.isEmpty() &&
standbyPartitionsOnHost.isEmpty())) {
Review Comment:
I'm trying to remember what the original intention behind this check was, I
imagine it was to avoid rebuilding the metadata unnecessarily and that we did
not have this check before named topologies because this only becomes
nontrivial as the metadata rebuild cost scales with the number of
topologies...?
That said, wouldn't we still want/need to update the metadata if it's become
empty? It seems like we should actually only ever skip it when the delta is
zero/empty, not when the current set is empty...but I guess that's the root of
this fix to begin with. Anyways we should err on the side of correctness rather
than performance issues, especially perf issues we haven't seen/confirmed to
exist
--
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]
