ableegoldman commented on code in PR #18111:
URL: https://github.com/apache/kafka/pull/18111#discussion_r1876844961
##########
streams/src/test/java/org/apache/kafka/streams/StreamsBuilderTest.java:
##########
@@ -1816,6 +1816,105 @@ public void
shouldWrapProcessorsForStreamTableJoinWithGracePeriod() {
assertThat(counter.numConnectedStateStores(), CoreMatchers.is(2));
}
+ @Test
+ public void
shouldWrapProcessorsForStreamStreamJoinWithSpuriousResultsFix() {
+ final Map<Object, Object> props = dummyStreamsConfigMap();
+ props.put(PROCESSOR_WRAPPER_CLASS_CONFIG,
RecordingProcessorWrapper.class);
+
+ final WrapperRecorder counter = new WrapperRecorder();
+ props.put(PROCESSOR_WRAPPER_COUNTER_CONFIG, counter);
+
+ final StreamsBuilder builder = new StreamsBuilder(new
TopologyConfig(new StreamsConfig(props)));
+
+ final KStream<String, String> stream1 = builder.stream("input-1",
Consumed.as("source-1"));
+ final KStream<String, String> stream2 = builder.stream("input-2",
Consumed.as("source-2"));
+
+ stream1.join(
+ stream2,
+ MockValueJoiner.TOSTRING_JOINER,
+ JoinWindows.ofTimeDifferenceAndGrace(Duration.ofDays(1),
Duration.ofDays(1)),
+ StreamJoined.as("ss-join"))
+ .to("output", Produced.as("sink"));
+
+ builder.build();
+
+ // TODO: fix these names once we address
https://issues.apache.org/jira/browse/KAFKA-18191
+ assertThat(counter.wrappedProcessorNames(),
Matchers.containsInAnyOrder(
+ "KSTREAM-JOINTHIS-0000000004", "KSTREAM-JOINOTHER-0000000005",
+ "KSTREAM-WINDOWED-0000000003", "KSTREAM-WINDOWED-0000000002",
+ "KSTREAM-MERGE-0000000006"
+ ));
+ assertThat(counter.numWrappedProcessors(), CoreMatchers.is(5));
+ assertThat(counter.numUniqueStateStores(), CoreMatchers.is(2));
+ assertThat(counter.numConnectedStateStores(), CoreMatchers.is(4));
+ }
+
+ @SuppressWarnings("deprecation")
+ @Test
+ public void
shouldWrapProcessorsForStreamStreamJoinWithoutSpuriousResultsFix() {
+ final Map<Object, Object> props = dummyStreamsConfigMap();
+ props.put(PROCESSOR_WRAPPER_CLASS_CONFIG,
RecordingProcessorWrapper.class);
+
+ final WrapperRecorder counter = new WrapperRecorder();
+ props.put(PROCESSOR_WRAPPER_COUNTER_CONFIG, counter);
+
+ final StreamsBuilder builder = new StreamsBuilder(new
TopologyConfig(new StreamsConfig(props)));
+
+ final KStream<String, String> stream1 = builder.stream("input-1",
Consumed.as("source-1"));
+ final KStream<String, String> stream2 = builder.stream("input-2",
Consumed.as("source-2"));
+
+ stream1.join(
+ stream2,
+ MockValueJoiner.TOSTRING_JOINER,
+ JoinWindows.of(Duration.ofDays(1)), // intentionally uses
deprecated version of this API!
+ StreamJoined.as("ss-join"))
+ .to("output", Produced.as("sink"));
+
+ builder.build();
+
+ // TODO: fix these names once we address
https://issues.apache.org/jira/browse/KAFKA-18191
+ assertThat(counter.wrappedProcessorNames(),
Matchers.containsInAnyOrder(
+ "KSTREAM-JOINTHIS-0000000004", "KSTREAM-JOINOTHER-0000000005",
+ "KSTREAM-WINDOWED-0000000003", "KSTREAM-WINDOWED-0000000002",
+ "KSTREAM-MERGE-0000000006"
+ ));
+ assertThat(counter.numWrappedProcessors(), CoreMatchers.is(5));
+ assertThat(counter.numUniqueStateStores(), CoreMatchers.is(2));
+ assertThat(counter.numConnectedStateStores(), CoreMatchers.is(4));
+ }
+
+ @Test
+ public void shouldWrapProcessorsForStreamStreamSelfJoin() {
+ final Map<Object, Object> props = dummyStreamsConfigMap();
+ props.put(PROCESSOR_WRAPPER_CLASS_CONFIG,
RecordingProcessorWrapper.class);
+
+ final WrapperRecorder counter = new WrapperRecorder();
+ props.put(PROCESSOR_WRAPPER_COUNTER_CONFIG, counter);
+
+ final StreamsBuilder builder = new StreamsBuilder(new
TopologyConfig(new StreamsConfig(props)));
+
+ final KStream<String, String> stream1 = builder.stream("input",
Consumed.as("source"));
+
+ stream1.join(
+ stream1,
+ MockValueJoiner.TOSTRING_JOINER,
+ JoinWindows.ofTimeDifferenceWithNoGrace(Duration.ofDays(1)),
+ StreamJoined.as("ss-join"))
+ .to("output", Produced.as("sink"));
+
+ builder.build();
+
+ // TODO: fix these names once we address
https://issues.apache.org/jira/browse/KAFKA-18191
+ assertThat(counter.wrappedProcessorNames(),
Matchers.containsInAnyOrder(
+ "KSTREAM-JOINTHIS-0000000003", "KSTREAM-JOINOTHER-0000000004",
+ "KSTREAM-WINDOWED-0000000001", "KSTREAM-WINDOWED-0000000002",
+ "KSTREAM-MERGE-0000000005"
+ ));
+ assertThat(counter.numWrappedProcessors(), CoreMatchers.is(5));
+ assertThat(counter.numUniqueStateStores(), CoreMatchers.is(2));
Review Comment:
I'm a little surprised by this because I thought we had a n optimization to
re-use the window store for self-joins. But I double checked against the actual
topology names and even before any of these changes, it's been this way 🤷♀️
(btw once we have everything finished and wrapped, maybe we should update
these tests to automatically validate the processor names against what's in the
topology builder -- rather than hard-coding them like this. or better yet, do
both)
--
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]
