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

Support for alternative thread pooling approaches for async processes? #111

Open
bturner opened this issue Apr 4, 2020 · 0 comments
Open

Comments

@bturner
Copy link
Collaborator

bturner commented Apr 4, 2020

Bitbucket Server 7.0 has shipped with NuProcess enabled by default, and the system uses it for the majority of the git processes it runs. Predominantly the system uses NuProcessBuilder.run, but it does sometimes use start to run them asynchronously. Most of the time, that works fine. However, we've found that, if any NuProcessHandler for an asynchronous process "misbehaves" by blocking for an unexpectedly long time, it basically causes a cascading failure that brings down the entire system and leaves zombie processes behind. (If the handler eventually becomes unblocked the system can recover, but we've seen cases where, for various reasons, the handler can never free up.) A misbehaving handler for 1 process can effectively kill a pump thread, meaning it no longer pumps any processes assigned to it.

Support for asynchronous processes is valuable, though, and so we want to have access to it. It's a great feature of NuProcess. We've done what we can to "harden" our handlers, to ensure they never block (or at most block only very briefly), but at this point I'm wondering whether you might be open to accepting a patch that made the way NuProcess handles its pump threads configurable. The default implementation could remain the current round-robin assignment across a fixed set of threads, but an alternative implementation (which is the one Bitbucket Server would use) would use something like an ExecutorService to pump a single process at once, with that thread potentially reused for a different (single) process after that process returned.

Such an approach is significantly "safer" for us, at the cost of additional threads. However, given we're switching away from an approach using Java's ProcessBuilder which requires 2-3 threads per process (plus the JVM-managed reaper thread), using a single thread per process with NuProcess still represents a considerable savings for the system.

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

No branches or pull requests

1 participant