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

fix: don't deadlock the runtime when fetching MSK auth token #803

Merged
merged 1 commit into from
Dec 5, 2024

Conversation

emef
Copy link
Contributor

@emef emef commented Dec 5, 2024

the previous code introduced a deadlock on the main tokio runtime (joining a thread that was running a blocking call on the runtime).

the only solution I found to this was to create a new temporary tokio runtime so that we can run the async aws code inside the mandatory non-async function. this function is only called when a kafka client using MSK auth needs to fetch a new oauth token, so it should be pretty infrequent.

Copy link
Member

@mwylde mwylde left a comment

Choose a reason for hiding this comment

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

This seems reasonable for now, but I think a better approach would be to run a separate task that fetches and refreshes the oauth token, and stores it in an Arc<Mutex<_>> which is read from the generate_oauth_token metho. Ideally we wouldn't ever be blocking the async thread with a blocking method like this.

The real fix is to change the API in librdkafka (for which there's an open issue: fede1024/rust-rdkafka#727) but the upstream project is somewhat dormant at the moment.

@mwylde mwylde merged commit 29e916f into ArroyoSystems:master Dec 5, 2024
3 checks passed
@mattforbes-gr
Copy link

This seems reasonable for now, but I think a better approach would be to run a separate task that fetches and refreshes the oauth token, and stores it in an Arc<Mutex<_>> which is read from the generate_oauth_token metho. Ideally we wouldn't ever be blocking the async thread with a blocking method like this.

The real fix is to change the API in librdkafka (for which there's an open issue: fede1024/rust-rdkafka#727) but the upstream project is somewhat dormant at the moment.

yeah I agree w/ all this. I think that the rdkafka library is calling the generate_oauth_token function from c so I'm sure it wouldn't be any easy thing to make async-friendly. the background refresh is the next best thing but I just wanted to get unblocked. if I have some time free up, I can take a pass at cleaning that up

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