-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Increase delay before checking the uv loop #51003
Conversation
This increases the delay slightly before checking whether all I/O tasks have finished. This may reduce the number of "spurious" warnings during precompilation. Fixes #50873
Note that #50914 is still needed to provide more guidance about the "non-spurious" cases. Also, I should acknowledge that 30ms may not be enough either; however, I imagine there's a cost to increasing it even more. Presumably we could do something akin to
before starting the timer at all (and then with an even longer delay before issuing the first warning), but I don't know the libuv/IO part of Julia at all and don't have time to learn what I'd need to learn to make such a change. |
In the spirit of trying to make the UX of the trailing timer UX feel less breaking, as well as this, what if the check had a timeout of say 30s, after which a warning was printed, but the process returned successfully
That way warnings are printed but precompilation doesn't hang indefinitely? |
If we did that you'd only need one warning before that, (rather than the current repeated one)
|
More reports of confusion here, where 3/4 package outputs look like they might be fixed by increasing this delay? (MKL_jll is clearly real as it's downloading) |
Also https://discourse.julialang.org/t/waiting-for-io-to-finish-1-10-beta2/103240 This has been brought up often enough that I'm tagging it as a milestone for 1.10. |
MKL_jll is not real, that is a Downloads.jl issue |
@vtjnash, what's your thought here? Is this a package bug (the package is responsible for |
Our current thinking is we can ask Julia for more information about the handle and try to get something extra from that to print |
That would be really, really nice. Some way to trace |
Since a delay of 30 ms (proposed here, and also current 10) is just arbitrary, but seemingly not too bad, should this just be merged? This is "mitigation (for a warning), so seems harmless, while not a "fix"? Is there a better fix, or bad affect with merging this, hiding some real problem? |
A couple of sources of spurious triggers for these messages have now been fixed (on master and next 1.10) so I think we could close this and see if the issue returns. As for the deadlock where the timer message loops, that seems to be #51662 |
This increases the delay slightly before checking whether all I/O
tasks have finished. This may reduce the number of "spurious" warnings
during precompilation.
Fixes #50873