vamossagar12 commented on code in PR #12803:
URL: https://github.com/apache/kafka/pull/12803#discussion_r1025489083
##########
streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamsMetadataState.java:
##########
@@ -459,12 +461,23 @@ private List<StreamsMetadata>
rebuildMetadataForSingleTopology(final Map<HostInf
return rebuiltMetadata;
}
+ private final Function<Optional<Set<Integer>>, Integer> getPartition =
maybeMulticastPartitions -> {
+ if (!maybeMulticastPartitions.isPresent()) {
+ return null;
+ }
+ if (maybeMulticastPartitions.get().size() != 1) {
+ throw new IllegalArgumentException("The partitions returned by
StreamPartitioner#partitions method when used for fetching KeyQueryMetadata for
key should be a singleton set");
Review Comment:
Thanks for the suggestion. Hmm I had assumed all along that even IQ would be
based off of a single partition. Infact we had discussed it during the KIP
design and this was a suggestion from Matthias:
```
Btw: The `StreamPartitioner` interface is also used for IQ. For both IQ
and FK-join, it seem ok to just add a runtime check that the returned
list is a singleton (in case we don't add a new class)?
```
But I can see a mismatch in the partitioners used. IIUC, the
`StreamPartitioner` used here is to be used for finding the partitioner for the
key and I am trying to enforce a single partition check here. But, I don't
think I added any such conditions at the time of writing the keys. So, that's
something which clearly can causes confusion and even yield wrong results and
looks like that's what you are also pointing to. So, would need to enhance the
`KeyQueryMetadata` class. And yeah, deprecating `partition()` method can be
avoided at this point.
On a different note, do you think a similar condition can happen even for FK
joins?
--
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]
