Skip to content

Allow groupID to be specified when assigning partition #161

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

Merged
merged 4 commits into from
Apr 25, 2024

Conversation

omarkj
Copy link
Contributor

@omarkj omarkj commented Jan 3, 2024

Motivation:

A Consumer Group can provide a lot of benefits even if the dynamic loadbalancing features are not used.

Modifications:

Allow for an optional GroupID when creating a partition consumer.

Result:

Consumer Groups can now be used when manual assignment is used.

@FranzBusch
Copy link
Contributor

@swift-server-bot test this please

/// - group: The ID of the consumer group to commit to.
/// - topic: The name of the Kafka topic.
/// - offset: The offset to start consuming from. Defaults to the end of the Kafka partition queue (meaning wait for the next produced message).
public static func partition(
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can merge this with the above case. Can we also spell out in the groupID doc comment when one would want to provide it when assigning the consumer to a specific partition?

Motivation:

A Consumer Group can provide a lot of benefits even if the
dynamic loadbalancing features are not used.

Modifications:

Allow for an optional GroupID when creating a partition
consumer.

Result:

Consumer Groups can now be used when manual assignment is
used.
@omarkj omarkj force-pushed the group-id-manual-assignment branch from 912a41e to 2ccacb9 Compare January 5, 2024 22:40
@omarkj omarkj requested a review from FranzBusch January 16, 2024 18:45
@FranzBusch
Copy link
Contributor

@swift-server-bot test this please

1 similar comment
@FranzBusch
Copy link
Contributor

@swift-server-bot test this please

Copy link
Collaborator

@blindspotbounty blindspotbounty left a comment

Choose a reason for hiding this comment

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

@omarkj thank you for the PR!
And sorry for the late review.. Could you rebase the branch to the latest main that CI would pass, please?

@blindspotbounty blindspotbounty enabled auto-merge (squash) March 27, 2024 09:43
@blindspotbounty
Copy link
Collaborator

@swift-server-bot test this please

1 similar comment
@FranzBusch
Copy link
Contributor

@swift-server-bot test this please

@blindspotbounty
Copy link
Collaborator

@swift-server-bot test this please

1 similar comment
@FranzBusch
Copy link
Contributor

@swift-server-bot test this please

@blindspotbounty blindspotbounty enabled auto-merge (squash) April 18, 2024 08:33
@FranzBusch FranzBusch disabled auto-merge April 25, 2024 19:34
@FranzBusch
Copy link
Contributor

@swift-server-bot test this please

@FranzBusch FranzBusch enabled auto-merge (squash) April 25, 2024 19:35
@FranzBusch FranzBusch merged commit 8be7e2f into swift-server:main Apr 25, 2024
blindspotbounty added a commit to ordo-one/swift-kafka-client that referenced this pull request Aug 6, 2024
* Feature: expose librdkafka statistics as swift metrics (swift-server#92)

* introduce statistics for producer

* add statistics to new consumer with events

* fix some artefacts

* adjust to KeyRefreshAttempts

* draft: statistics with metrics

* make structures internal

* Update Sources/Kafka/Configuration/KafkaConfiguration+Metrics.swift

Co-authored-by: Felix Schlegel <[email protected]>

* Update Sources/Kafka/Configuration/KafkaConsumerConfiguration.swift

Co-authored-by: Felix Schlegel <[email protected]>

* Update Sources/Kafka/Configuration/KafkaConfiguration+Metrics.swift

Co-authored-by: Felix Schlegel <[email protected]>

* Update Sources/Kafka/Configuration/KafkaConfiguration+Metrics.swift

Co-authored-by: Felix Schlegel <[email protected]>

* address review comments

* formatting

* map gauges in one place

* move json mode as rd kafka statistics, misc renaming + docc

* address review comments

* remove import Metrics

* divide producer/consumer configuration

* apply swiftformat

* fix code after conflicts

* fix formatting

---------

Co-authored-by: Felix Schlegel <[email protected]>

* Add benchmark infratructure without actual tests (swift-server#146)

* add benchmark infratructure without actual test

* apply swiftformat

* fix header in sh file

* use new async seq methods

* Update to latest librdkafka & add a define for RAND_priv_bytes (swift-server#148)

Co-authored-by: Franz Busch <[email protected]>

* exit from consumer batch loop when no more messages left (swift-server#153)

* Lower requirements for consumer state machine (swift-server#154)

* lower requirements for kafka consumer

* add twin test for kafka producer

* defer source.finish (swift-server#157)

* Add two consumer benchmark (swift-server#149)

* benchmark for consumer

* attempty to speedup benchmarks

* check CI works for one test

* enable one more test

* try to lower poll interval

* adjust max duration of test

* remain only manual commit test

* check if commit is the reason for test delays

* try all with schedule commit

* revert max test time to 5 seconds

* dockerfiles

* test set threasholds

* create dummy thresholds from ci results

* disable benchmark in CI

* add header

* add stable metrics

* update thresholds to stable metrics only

* try use '1' instead of 'true'

* adjust thresholds to CI results (as temporary measure)

* set 20% threshold..

* move arc to unstable metrics

* try use 'true' in quotes for CI

* try reduce number of messages for more reliable results

* try upgrade bench

* disable benchmark in CI

* Update librdkafka for BoringSSL (swift-server#162)

* chore(patch): [sc-8379] use returned error (swift-server#163)

* [producer message] Allow optional key for initializer (swift-server#164)

Co-authored-by: Harish Yerra <[email protected]>

* Allow groupID to be specified when assigning partition (swift-server#161)

* Allow groupID to be specified when assigning partition

Motivation:

A Consumer Group can provide a lot of benefits even if the
dynamic loadbalancing features are not used.

Modifications:

Allow for an optional GroupID when creating a partition
consumer.

Result:

Consumer Groups can now be used when manual assignment is
used.

* fix format

---------

Co-authored-by: Ómar Kjartan Yasin <[email protected]>
Co-authored-by: blindspotbounty <[email protected]>
Co-authored-by: Franz Busch <[email protected]>

* Wrap rd_kafka_consumer_poll into iterator (use librdkafka embedded backpressure) (swift-server#158)

* remove message sequence

* test consumer with implicit rebalance

* misc + format

* remove artefact

* don't check a lot of messages

* fix typo

* slow down first consumer to lower message to fit CI timeout

* remove helpers

* use exact benchmark version to avoid missing thresholds error (as no thresholds so far)

* add deprecated marks for backpressure, change comment for future dev

* address comments

---------

Co-authored-by: Felix Schlegel <[email protected]>
Co-authored-by: Axel Andersson <[email protected]>
Co-authored-by: Franz Busch <[email protected]>
Co-authored-by: Samuel M <[email protected]>
Co-authored-by: Harish Yerra <[email protected]>
Co-authored-by: Harish Yerra <[email protected]>
Co-authored-by: Omar Yasin <[email protected]>
Co-authored-by: Ómar Kjartan Yasin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants