New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow kafka-streams binder to consume messages from multiple kafka topics #362

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
5 participants
@sarathshyam
Copy link
Contributor

sarathshyam commented Apr 14, 2018

This is a proposal to fix the issue #361.
Changes

  • Modified org.springframework.cloud.stream.binder.kafka.streams.KafkaStreamsStreamListenerSetupMethodOrchestrator#getkStream so that the KStream object is built from list of topic names.
  • Added new test case org.springframework.cloud.stream.binder.kafka.streams.KafkaStreamsBinderMultipleInputTopicsTest

Fixes #361

sarathshyam added some commits Apr 14, 2018

Fix kaka-streams binder to consume messages from multiple input topics
…#361

Modified KafkaStreamsStreamListenerSetupMethodOrchestrator#getkStream
so that the KStream object is built from list of topic names

@sabbyanandan sabbyanandan added the in pr label Apr 14, 2018

@garyrussell

This comment has been minimized.

Copy link
Contributor

garyrussell commented Apr 14, 2018

While this might work fine for the Kafka Streams binder, I think there needs to be a more general solution for all binders that can support consuming from multiple topics (kafka, rabbitmq etc message channel binders, where the listener containers can be configured to consume from a lis of topics/queues).

@olegz

This comment has been minimized.

Copy link
Contributor

olegz commented Apr 16, 2018

I agree with @garyrussell and there is actually an issue that we already have for it spring-cloud/spring-cloud-stream#1168

@sobychacko

This comment has been minimized.

Copy link
Contributor

sobychacko commented Apr 23, 2018

Hi @sarathshyam - We are actively reviewing this PR. We might have to take a slightly different approach or add some extra changes at the core framework level to address this scenario. We will update soon on this PR about any progress.

@sarathshyam

This comment has been minimized.

Copy link
Contributor Author

sarathshyam commented Apr 24, 2018

@sobychacko

This comment has been minimized.

Copy link
Contributor

sobychacko commented May 1, 2018

@sarathshyam Thank you for the PR. This is merged upstream. A couple of notes related to this change. We introduced a new property at the core consumer level called multiplex. When it is true, the binder natively is capable of consuming from multiple topics through a single input binding. This is always made true in the kafka streams binder. See the changes at this commit: spring-cloud/spring-cloud-stream@61bce52

Once this change was in the core framework, I added two more commits on top of your commit in the kafka streams binder. The following changes are introduced through those 2 commits.

  • Spring Cloud Stream is updated to 2.1.0.BUILD-SNAPSHOT
  • multiplex property is made true always
  • When we have multiple inputs, the DLQ handling in kafka streams binder needs to be adjusted differently, so that we can have separate dlq topic per input topic or a catch all DLQ topic specified through the dlq name. These changes are also addressed through a commit in the PR.

I did not squash the 2 commits that I added in the PR as they have individual focus but related to your base commit.

Once again, thank you. Looking forward to more PR's in the future!

@sobychacko sobychacko closed this May 1, 2018

@sobychacko sobychacko removed the in pr label May 1, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment