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

Allow multiple brokers in kafka executor #1037

Merged

Conversation

Espina2
Copy link
Contributor

@Espina2 Espina2 commented Jan 13, 2022

Kafka is supposed to be distributed, passing only broker can make messages fail if for some reason the broker is not up. In order to circumvent that issue, we can use multiple brokers. This can also improve the publishing rate.

Signed-off-by: Paulo Moura itsme@paulomoura.com.pt

@Espina2 Espina2 force-pushed the kafka-executor-multiple-brokers branch 6 times, most recently from 2690b08 to acfcfb3 Compare January 15, 2022 12:17
@Espina2
Copy link
Contributor Author

Espina2 commented Jan 15, 2022

@vcastellm can you take look?

Copy link
Collaborator

@yvanoers yvanoers left a comment

Choose a reason for hiding this comment

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

Nice, thanks! I have a couple of questions, though.

builtin/bins/dkron-executor-kafka/kafka.go Show resolved Hide resolved
website/content/usage/executors/kafka.md Outdated Show resolved Hide resolved
@Espina2 Espina2 force-pushed the kafka-executor-multiple-brokers branch from acfcfb3 to cd9417a Compare January 15, 2022 23:52
vcastellm
vcastellm previously approved these changes Jan 16, 2022
Copy link
Member

@vcastellm vcastellm left a comment

Choose a reason for hiding this comment

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

LGTM

@Espina2
Copy link
Contributor Author

Espina2 commented Jan 16, 2022

LGTM

This are failing tough

@vcastellm
Copy link
Member

LGTM

This are failing tough

I'm checking the CI

@Espina2
Copy link
Contributor Author

Espina2 commented Jan 16, 2022

Let me know if there is anything I can help

@vcastellm
Copy link
Member

SSL tests certs updated, rebase with master and it should work @Espina2

Signed-off-by: Paulo Moura <itsme@paulomoura.com.pt>
@Espina2
Copy link
Contributor Author

Espina2 commented Jan 16, 2022

@vcastellm done :)

Copy link
Member

@vcastellm vcastellm left a comment

Choose a reason for hiding this comment

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

LGTM

@vcastellm vcastellm merged commit 04696f1 into distribworks:master Jan 16, 2022
@Espina2 Espina2 deleted the kafka-executor-multiple-brokers branch January 22, 2022 18:45
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.

None yet

3 participants