-
Notifications
You must be signed in to change notification settings - Fork 17.7k
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
path/filepath: add Glob variant taking a context.Context? #16448
Comments
That seems like a domain-crossing change. It makes me uncomfortable. Lots of things take a long time, and it would be lousy if every possible long-running operation got a Context variant. |
Perhaps a more general cancellation mechanism than contexts is in order ? It'd be very nice if most long running operations in Go could be cancelled, but I don't think context is the right tool for the job. |
Which part of Glob is taking a long time? If it's the syscall, then we
cannot do anything about it even if Glob supports cancellation. (Sending
signals might not work if the process is in uninterruptible state due to
underlying file system.)
|
Now that we have the context package in the standard library, I don't think we want to introduce another, different, cancellation mechanism. I don't know but I wouldn't guess that the problem is the system call. I suspect the problem is that I don't think it makes sense to add a |
@minux POSIX says all file system calls that block indeterminately due to waiting on the underlying disk/network storage device are interruptible (though I would expect in practice that kernel bugs and things like FUSE regularly break that). |
I frequently encounter processes in D state due to flaky NFS in the past.
Our Glob doesn't support **, so even if it's traversaling huge directories,
if the majority time is not spent in syscall and GC, I think we should
optimize Glob more.
|
Yeah, I assumed our glob supported "**" when I filed the bug because I remembered #11862 and thought it was already fixed. We can do nothing here. The internal Google glob can be fixed instead. |
From internal bug report:
filepath.Glob can take a lot of time, and can't be canceled.
Add a Context variant?
The text was updated successfully, but these errors were encountered: