Skip to content
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

Migrate Notification Publisher to Confluent Parallel Consumer #586

Merged
merged 9 commits into from
Jun 7, 2023

Conversation

nscuro
Copy link
Member

@nscuro nscuro commented May 28, 2023

Because it does not need any of the features provided by Kafka Streams, it doesn't necessarily make sense to use KS.

I looked into migrating to SmallRye messaging (the "native" Quarkus solution), but it still has unpleasant drawbacks that make it not a good fit (#215 (comment)). Most notably, ordered, blocking processing will process all events on a single thread, no matter how many partitions the input topics have. This is prone to become a huge bottleneck under high load.

With Parallel Consumer, we get ordering guarantees by key and still almost unlimited concurrency (https://github.com/confluentinc/parallel-consumer#ordered-by-key). Further, processing is decoupled from the actual number of partitions.

A nice side-effect is also that it supports retries (https://github.com/confluentinc/parallel-consumer#retries).

Relates to #346

It doesn't play well with Kafka Streams.

Signed-off-by: nscuro <nscuro@protonmail.com>
Because it does not need any of the features provided by Kafka Streams, it doesn't necessarily make sense to use KS.

I looked into migrating to SmallRye messaging (the "native" Quarkus solution), but it still has unpleasant drawbacks that make it not a good fit (#215 (comment)). Most notably, ordered, blocking processing will process all events on a single thread, no matter how many partitions the input topics have. This is prone to become a huge bottleneck under high load.

With Parallel Consumer, we get ordering guarantees by key and *still* almost unlimited concurrency (https://github.com/confluentinc/parallel-consumer#ordered-by-key). Further, processing is decoupled from the actual number of partitions.

A nice side-effect is also that it supports retries (https://github.com/confluentinc/parallel-consumer#retries).

Signed-off-by: nscuro <nscuro@protonmail.com>
@nscuro nscuro added enhancement New feature or request domain/notifications labels May 28, 2023
@nscuro nscuro changed the title Migrate np to parallel consumer Migrate Notification Publisher to Confluent Parallel Consumer May 28, 2023
sahibamittal and others added 4 commits May 29, 2023 14:19
commit 1d6c7f1
Author: Niklas <nscuro@protonmail.com>
Date:   Mon May 29 11:42:01 2023 +0200

    Revise labels in Helm chart (#583)

    * Properly truncate `name` and `fullname` such that they never exceed 63 characters, even with suffixes
    * Add `app.kubernetes.io/part-of` label
    * Make `app.kubernetes.io/name` specific to individual services, instead of using a common label values across all services
    * Use fully qualified names in `metadata.name`

    Signed-off-by: nscuro <nscuro@protonmail.com>

commit 4f3f6fe
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Mon May 29 10:40:29 2023 +0100

    Bump checkstyle from 10.11.0 to 10.12.0 (#587)

commit d209dd2
Author: Niklas <nscuro@protonmail.com>
Date:   Mon May 29 11:39:32 2023 +0200

    Replace Flyway with testcontainers init script (#585)
It doesn't play well with Kafka Streams.

Signed-off-by: nscuro <nscuro@protonmail.com>
Because it does not need any of the features provided by Kafka Streams, it doesn't necessarily make sense to use KS.

I looked into migrating to SmallRye messaging (the "native" Quarkus solution), but it still has unpleasant drawbacks that make it not a good fit (#215 (comment)). Most notably, ordered, blocking processing will process all events on a single thread, no matter how many partitions the input topics have. This is prone to become a huge bottleneck under high load.

With Parallel Consumer, we get ordering guarantees by key and *still* almost unlimited concurrency (https://github.com/confluentinc/parallel-consumer#ordered-by-key). Further, processing is decoupled from the actual number of partitions.

A nice side-effect is also that it supports retries (https://github.com/confluentinc/parallel-consumer#retries).

Signed-off-by: nscuro <nscuro@protonmail.com>
@nscuro nscuro force-pushed the migrate-np-to-parallel-consumer branch from 8667931 to a2a9a6f Compare May 30, 2023 13:06
sahibamittal
sahibamittal previously approved these changes Jun 2, 2023
Copy link
Collaborator

@sahibamittal sahibamittal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@sonarcloud
Copy link

sonarcloud bot commented Jun 6, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 5 Code Smells

47.7% 47.7% Coverage
0.0% 0.0% Duplication

@sahibamittal
Copy link
Collaborator

Passing the Sonar Quality code coverage because coverage seems good locally and is optimal.

@sahibamittal sahibamittal merged commit 1e20c56 into main Jun 7, 2023
11 of 12 checks passed
@sahibamittal sahibamittal deleted the migrate-np-to-parallel-consumer branch June 7, 2023 11:00
nscuro added a commit that referenced this pull request Jul 2, 2023
As of #586, concurrency of notification publishing is not directly coupled to partition count anymore. For local testing and demo purposes, there's no need to use more than one partition.

Signed-off-by: nscuro <nscuro@protonmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants