Add a concurrency limit on FileWriter ops. #9857
Conversation
FileWriters are used to write artifacts to disk by the cache implementations. When there are bursts of file writer operations, it can cause the Go runtime to create an unbounded number of system threads as file writer ops end up blocked on syscalls. The concurrency limit places an upper bound on the system threads created via this code path.
| if err != nil { | ||
| return nil, err | ||
| } | ||
| defer releaseQuota() |
There was a problem hiding this comment.
I'm a bit conflicted here. In some ways, I think it would be better to get quota once and hold on to it until the writer is closed. That way, there's less overhead for getting quota, plus we would prefer to finish ongoing writes instead of starting new ones.
On the other hand, it seems bad to hold on to the quota while we're not doing anything, or maybe just receiving the next chunk from the client.
Anyway, this seems ok as is, unless there's some benchmark that shows a proble.
There was a problem hiding this comment.
The only limit we are currently encountering in practice is on the number of blocked syscalls which is what this PR is trying to address. Having a limit on the total number of writers also makes sense, but I think that needs to be handled and configured separately from this limit.
FileWriters are used to write artifacts to disk by the cache implementations. When there are bursts of file writer operations, it can cause the Go runtime to create an unbounded number of system threads as file writer ops end up blocked on syscalls. The concurrency limit places an upper bound on the system threads created via this code path. Fixes: buildbuddy-io/buildbuddy-internal#5325
FileWriters are used to write artifacts to disk by the cache implementations. When there are bursts of file writer operations, it can cause the Go runtime to create an unbounded number of system threads as file writer ops end up blocked on syscalls.
The concurrency limit places an upper bound on the system threads created via this code path.
Fixes: https://github.com/buildbuddy-io/buildbuddy-internal/issues/5325