-
Notifications
You must be signed in to change notification settings - Fork 74
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
Feature/kafka properties for source #618
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
yl-lisen
reviewed
Mar 20, 2024
failed smoke
|
yl-lisen
reviewed
Mar 20, 2024
yl-lisen
reviewed
Mar 20, 2024
yl-lisen
reviewed
Mar 20, 2024
zliang-min
force-pushed
the
feature/kafka-properties-for-source
branch
from
March 20, 2024 21:30
ca6dc4f
to
242f15c
Compare
yl-lisen
reviewed
Mar 21, 2024
yl-lisen
reviewed
Mar 21, 2024
yl-lisen
reviewed
Mar 21, 2024
yl-lisen
reviewed
Mar 21, 2024
yl-lisen
reviewed
Mar 21, 2024
yl-lisen
approved these changes
Mar 21, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
(Jove Github Bot) added it to the current sprint. |
(Jove Github Bot) moved this ticket out of the GitHub project(up to 1200 tickets for one project). |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR checklist:
proton: starts/ends
for new code in existing community code base ?Please write user-readable short description of the changes:
This PR has completely rewritten how producers and consumers are managed in Kafka external streams.
First, all librdkafka resource management code (for external stream) have been moved to the
RdKafka
namespace. In Kafka external stream implementaion, it no longer uses the code from theklog
namespace (directly). The goal is that, eventually, the Kafka external stream implementation andklog
will all useRdKafka
for communicating with Kafka via the librdkafka library. Andklog
will only be used the Proton internal Kafka log stream.With this PR, resources from librdkafka will be shared by multiple sources/sinks of the same external stream as much as possible. This will reduce resource consumption compared to the current implementation. Esp. when there are a lot of topics and topics have a lot partitions. refers #544
This also fixed the resource leakage issue with the current pooling implementation. closes #236
This also fixed external stream won't pick up new paritions when used as a source. closes #291 ( Note that, a running MV or query that selects from an external stream won't automatically updated to pick up the new paritions, it needs to be re-created or re-run ).
This also fixed the issue that the
properties
setting does not work for external stream source. closes #543This also fixed the issue that an external stream still gets created even some settings are not correct. closes #617
This also fixed an issue when using
ProtobufSingle
to write multiple rows of data to an external stream. (no ticket)