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.

Sign up for GitHub

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

Jump to bottom

Refactor KafkaSink #205

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Refactor KafkaSink #205

wants to merge 1 commit into from

Conversation

mejroslav
Copy link
Collaborator

@mejroslav mejroslav commented Jul 25, 2024 •
edited
Loading

  • There is no default topic name. Runtime Error is raised when topic or bootstrap.servers are not provided.
  • poll function removed from on_tick
  • It is possible to unthrottle the pipeline in process

@mejroslav mejroslav self-assigned this Jul 25, 2024
PremyslCerny
PremyslCerny reviewed Jul 25, 2024
View reviewed changes
Copy link
Collaborator

@PremyslCerny PremyslCerny left a comment

Choose a reason for hiding this comment

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

Please add comments to each change, like why we need the on tick etc.

PremyslCerny
PremyslCerny reviewed Jul 26, 2024
View reviewed changes
bspump/kafka/sink.py

def _kafka_poll(self):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add unthrottle back to kafka_poll.

ateska
ateska requested changes Jul 30, 2024
View reviewed changes
Copy link
Contributor

@ateska ateska left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Change in the comment.

Also, please test a throughput before merging.
It must be in >200k EPS ...

bspump/kafka/sink.py
"batch.num.messages": "100000",
"linger.ms": "500", # This settings makes a significant impact on the throughtput
"batch.size": "1000000",
"poll.timeout": "0.1",
# "compression.type": "snappy",

# Custom options
Copy link
Contributor

Choose a reason for hiding this comment

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

Options that are not for librdkafka ...

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

@PremyslCerny PremyslCerny PremyslCerny left review comments

@ateska ateska ateska requested changes

Assignees

@mejroslav mejroslav

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mejroslav @ateska @PremyslCerny