-
Notifications
You must be signed in to change notification settings - Fork 13k
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
QNX Neutrino: exponential backoff when fork/spawn needs a retry #109432
QNX Neutrino: exponential backoff when fork/spawn needs a retry #109432
Conversation
Hey! It looks like you've submitted a new PR for the library teams! If this PR contains changes to any Examples of
|
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.
Sorry about the long turnaround time, April was apparently not great for thinking.
delay *= 2; | ||
continue; |
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 would like it if we still bailed for ErrorKind::WouldBlock
or ErrorKind::Interrupted
after a while, probably if we're getting up to actual seconds between yields.
@rustbot author |
Thank you very much @workingjubilee for the good feedback! Returning |
9b71cad
to
716cc5a
Compare
Requested changes have been implemented but should be tested before merging them. Branch is rebased; requested changes are in a separate commit to make the review easier. |
r? @workingjubilee |
Could not assign reviewer from: |
@rustbot review |
@workingjubilee What do you think about giving up after one second? Does it make sense to document this somewhere as QNX Neutrino specific behavior? Where? |
Yeah, 1 second as the upper bar seems fine. If you wish to document it in the platform support (for now? maybe we should have a "platform quirks" page somewhere...) page, then go ahead. It's an unlikely circumstance, even under kernel stress, that I think having this surface in Rust programs as an unhandled "glitch" is fine, i.e. real programs usually won't hit this. However, I feel I should note that EAGAIN is actually specifically defined as a plausible POSIX error for posix_spawn due to it including "all the reasons fork/exec would fail", and fork can EAGAIN. |
Given that, it might actually make more sense to document this as a possible error for all platforms? But any changes for documenting spawn's errors for all platforms should be in another PR, and you might want to document it as specifically likely for Neutrino anyways. |
Yes I think this should be documented in another PR. With this patch I wasn't able at all to reproduce the issue anymore (before the |
Sure! Then let's go with this. @bors r+ |
☀️ Test successful - checks-actions |
Finished benchmarking commit (1e17cef): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 647.289s -> 647.076s (-0.03%) |
Fixes #108594: When retrying, sleep with an exponential duration. When sleep duration is lower than minimum possible sleeping time, yield instead (this will not be often due to the exponential increase of duration).
Minimum possible sleeping time is determined using
libc::clock_getres
but only when spawn/fork failed the first time in a request. This is cached using a LazyLock.CC @gh-tr
r? @workingjubilee
@rustbot label +O-neutrino