You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2020/12/01 19:39:02 UTC

[GitHub] [kafka] mjsax opened a new pull request #9668: MINOR: add test for repartition/source-topic/changelog optimization

mjsax opened a new pull request #9668:
URL: https://github.com/apache/kafka/pull/9668


   If topology optimization is enabled, KafkaStreams does not create store
   changelog topics but re-uses source input topics if possible. However,
   this optimization should not be applied to internal repartition topics,
   because those are actively purged.
   
   Call for review @ableegoldman 


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [kafka] mjsax commented on a change in pull request #9668: MINOR: add test for repartition/source-topic/changelog optimization

Posted by GitBox <gi...@apache.org>.
mjsax commented on a change in pull request #9668:
URL: https://github.com/apache/kafka/pull/9668#discussion_r536256494



##########
File path: streams/src/test/java/org/apache/kafka/streams/StreamsBuilderTest.java
##########
@@ -463,7 +463,36 @@ public void shouldReuseSourceTopicAsChangelogsWithOptimization20() {
     }
 
     @Test
-    public void shouldNotReuseSourceTopicAsChangelogsByDefault() {
+    public void shouldNotReuseRepartitionTopicAsChangelogs() {
+        final String topic = "topic";
+        builder.<Long, String>stream(topic).repartition().toTable(Materialized.as("store"));
+        final Properties props = StreamsTestUtils.getStreamsConfig("appId");
+        props.put(StreamsConfig.TOPOLOGY_OPTIMIZATION_CONFIG, StreamsConfig.OPTIMIZE);
+        final Topology topology = builder.build(props);
+
+        final InternalTopologyBuilder internalTopologyBuilder = TopologyWrapper.getInternalTopologyBuilder(topology);
+        internalTopologyBuilder.rewriteTopology(new StreamsConfig(props));
+
+        assertThat(
+            internalTopologyBuilder.buildTopology().storeToChangelogTopic(),
+            equalTo(Collections.singletonMap("store", "appId-store-changelog"))
+        );
+        assertThat(
+            internalTopologyBuilder.stateStores().keySet(),
+            equalTo(Collections.singleton("store"))
+        );
+        assertThat(
+            internalTopologyBuilder.stateStores().get("store").loggingEnabled(),
+            equalTo(true)
+        );
+        assertThat(
+            internalTopologyBuilder.topicGroups().get(1).stateChangelogTopics.keySet(),
+            equalTo(Collections.singleton("appId-store-changelog"))
+        );
+    }
+
+    @Test
+    public void shouldNotReuseRepartitionTopicAsChangelogsByDefault() {

Review comment:
       Ah. Yes, the test name should not change.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [kafka] mjsax merged pull request #9668: MINOR: add test for repartition/source-topic/changelog optimization

Posted by GitBox <gi...@apache.org>.
mjsax merged pull request #9668:
URL: https://github.com/apache/kafka/pull/9668


   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [kafka] ableegoldman commented on a change in pull request #9668: MINOR: add test for repartition/source-topic/changelog optimization

Posted by GitBox <gi...@apache.org>.
ableegoldman commented on a change in pull request #9668:
URL: https://github.com/apache/kafka/pull/9668#discussion_r535724656



##########
File path: streams/src/test/java/org/apache/kafka/streams/StreamsBuilderTest.java
##########
@@ -463,7 +463,36 @@ public void shouldReuseSourceTopicAsChangelogsWithOptimization20() {
     }
 
     @Test
-    public void shouldNotReuseSourceTopicAsChangelogsByDefault() {
+    public void shouldNotReuseRepartitionTopicAsChangelogs() {
+        final String topic = "topic";
+        builder.<Long, String>stream(topic).repartition().toTable(Materialized.as("store"));
+        final Properties props = StreamsTestUtils.getStreamsConfig("appId");
+        props.put(StreamsConfig.TOPOLOGY_OPTIMIZATION_CONFIG, StreamsConfig.OPTIMIZE);
+        final Topology topology = builder.build(props);
+
+        final InternalTopologyBuilder internalTopologyBuilder = TopologyWrapper.getInternalTopologyBuilder(topology);
+        internalTopologyBuilder.rewriteTopology(new StreamsConfig(props));
+
+        assertThat(
+            internalTopologyBuilder.buildTopology().storeToChangelogTopic(),
+            equalTo(Collections.singletonMap("store", "appId-store-changelog"))
+        );
+        assertThat(
+            internalTopologyBuilder.stateStores().keySet(),
+            equalTo(Collections.singleton("store"))
+        );
+        assertThat(
+            internalTopologyBuilder.stateStores().get("store").loggingEnabled(),
+            equalTo(true)
+        );
+        assertThat(
+            internalTopologyBuilder.topicGroups().get(1).stateChangelogTopics.keySet(),
+            equalTo(Collections.singleton("appId-store-changelog"))
+        );
+    }
+
+    @Test
+    public void shouldNotReuseRepartitionTopicAsChangelogsByDefault() {

Review comment:
       This should still be called `shouldNotReuseSourceTopicAsChangelogsByDefault`,  right?




----------------------------------------------------------------
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:
users@infra.apache.org