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

Shift to user provided context shift #135

Merged

Conversation

antonlin1
Copy link
Contributor

user provided ContextShift is not used. Blaze selector is
executing user-level code. Added contextShift.shift to fix

user provided `ContextShift` is not used. Blaze selector is
executing user-level code. Added `contextShift.shift` to fix
@antonlin1
Copy link
Contributor Author

it was noted by @dufrannea that shifting this way makes us use user-level thread pools to write to sockets (performing IO work). This should not happen (to be investigated?), we need to get a hold underlying blaze thread pool and shift back to this one before performing IO operations.

@bubblesly
Copy link
Contributor

After investigation we see that:

  • the blaze thread pool only launches the requests processing
  • if we shift nothing prevents it to process other requests
  • if we shift we will indeed finish the processing on the user provided context. It's not ideal because the socket will be created on the blaze thread pool and used in another but I don't think it can be harmful for the app.

Furthermore I think that the original intent was to shift to the user code as this comment explains:
" // We use synchronouse queue for inbound & outbound trafic, so
// backpressure is fully managed by user code."

@antonlin1 antonlin1 merged commit f0f9e45 into criteo:master Mar 19, 2021
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

2 participants