-
Notifications
You must be signed in to change notification settings - Fork 590
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
fix(connector): unblock fetch_watermarks
in rdkafka
#15461
Conversation
Signed-off-by: Runji Wang <[email protected]>
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 as no business logic change
Signed-off-by: Runji Wang <[email protected]>
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.
I'm not sure if I understand it correctly:
- Upgrading rdkafka (chore(deps): update librdkafka to v2.3.0 #15313) already fixed the hanging issue
- This PR (
spawan_blocking
) isn't a must-have now, but it can avoid hanging even if there's bug in rdkafka. (can make async timeout work)
Right?
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.
Correct
Meta node exited with this error:
Don't know why. 😕 |
Merge queue setting changed
seems a network issue? not sure rerun helps |
got an error from inner
seems we break the rdkafka inner @wangrunji0408 |
Humm, looks like the raw kafka client can not be sent to another thread. But librdkafka claims it can be called from any thread.
|
Seems a kafka issue. Let's leave this in backlog. |
https://github.com/madsim-rs/madsim/pull/196/files?diff=split#r1535237591 👀 |
Signed-off-by: Runji Wang <[email protected]>
0127813
to
536b96a
Compare
Signed-off-by: Runji Wang <[email protected]>
Any reason this pr not merged? |
Signed-off-by: Runji Wang <[email protected]> Co-authored-by: Bohan Zhang <[email protected]> Signed-off-by: MrCroxx <[email protected]>
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
This PR bumps madsim-rdkafka. Operations in method
fetch_watermarks
is now wrapped intokio::task::spawn_blocking
to avoid blocking the current thread. https://github.com/madsim-rs/madsim/pull/196/files https://github.com/madsim-rs/madsim/pull/200/filesChecklist
./risedev check
(or alias,./risedev c
)Documentation
Release note
If this PR includes changes that directly affect users or other significant modifications relevant to the community, kindly draft a release note to provide a concise summary of these changes. Please prioritize highlighting the impact these changes will have on users.