mjsax commented on a change in pull request #8504:
URL: https://github.com/apache/kafka/pull/8504#discussion_r418818822
##########
File path:
streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamImpl.java
##########
@@ -989,16 +994,18 @@ private void to(final TopicNameExtractor<K, V>
topicExtractor,
null,
optimizableRepartitionNodeBuilder);
- final OptimizableRepartitionNode<K, V> optimizableRepartitionNode =
optimizableRepartitionNodeBuilder.build();
- builder.addGraphNode(streamsGraphNode, optimizableRepartitionNode);
+ if (repartitionNode == null || !name.equals(repartitionName)) {
Review comment:
> We probably should have a consistent approach. How about I make the
changes in this PR for the Join repartition topics (incrementing the index of
the repartition node name) and do a follow-on PR to address the other
repartition topics?
@bbejeck Works for me.
> I'm on the fence about whether this is an "optimization" or "reasonable
behavior".
@bbejeck @vvcephei That was my reasoning from my other comments about "don't
make the same mistake again", too (cf.
https://github.com/apache/kafka/pull/8504#issuecomment-618073161). Atm, we just
need to keep the old behavior for backward compatibility reasons and only give
"reasonable" behavior via opt-in to the optimization. IMHO, optimization should
be the default behavior anyway (not the other way round; we just have it that
way due to compatibility constraints) and you should even be able to turn it
off (if possible)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
