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

Remote: Don't blocking-get when acquiring gRPC connections. #14416

Closed
wants to merge 1 commit into from

Conversation

coeuvre
Copy link
Member

@coeuvre coeuvre commented Dec 13, 2021

With recent change to limit the max number of gRPC connections by default, acquiring a connection could suspend a thread if there is no available connection.

gRPC calls are scheduled to a dedicated background thread pool. Workers in the thread pool are responsible to acquire the connection before starting the RPC call.

There could be a race condition that a worker thread handles some gRPC calls and then switches to a new call which will acquire new connections. If the number of connections reaches the max, the worker thread is suspended and doesn't have a chance to switch to previous calls. The connections held by previous calls are, hence, never released.

This PR changes to not use blocking get when acquiring gRPC connections.

Fixes #14363.

@coeuvre coeuvre requested a review from philwo December 13, 2021 19:48
@coeuvre coeuvre requested a review from a team as a code owner December 13, 2021 19:48
@brentleyjones
Copy link
Contributor

brentleyjones commented Dec 13, 2021

Since 8d5973d is in the 5.0 RC, can we get this cherry-picked into the release as well? @Wyverald

Edit: I see that issue is marked as a release blocker, so we are fine. Sorry for the noise.

@bazel-io bazel-io closed this in ad663a7 Dec 14, 2021
coeuvre added a commit to coeuvre/bazel that referenced this pull request Dec 14, 2021
With recent change to limit the max number of gRPC connections by default, acquiring a connection could suspend a thread if there is no available connection.

gRPC calls are scheduled to a dedicated background thread pool. Workers in the thread pool are responsible to acquire the connection before starting the RPC call.

There could be a race condition that a worker thread handles some gRPC calls and then switches to a new call which will acquire new connections. If the number of connections reaches the max, the worker thread is suspended and doesn't have a chance to switch to previous calls. The connections held by previous calls are, hence, never released.

This PR changes to not use blocking get when acquiring gRPC connections.

Fixes bazelbuild#14363.

Closes bazelbuild#14416.

PiperOrigin-RevId: 416282883
brentleyjones pushed a commit to brentleyjones/bazel that referenced this pull request Dec 14, 2021
With recent change to limit the max number of gRPC connections by default, acquiring a connection could suspend a thread if there is no available connection.

gRPC calls are scheduled to a dedicated background thread pool. Workers in the thread pool are responsible to acquire the connection before starting the RPC call.

There could be a race condition that a worker thread handles some gRPC calls and then switches to a new call which will acquire new connections. If the number of connections reaches the max, the worker thread is suspended and doesn't have a chance to switch to previous calls. The connections held by previous calls are, hence, never released.

This PR changes to not use blocking get when acquiring gRPC connections.

Fixes bazelbuild#14363.

Closes bazelbuild#14416.

PiperOrigin-RevId: 416282883
(cherry picked from commit ad663a7)
Wyverald pushed a commit that referenced this pull request Dec 14, 2021
With recent change to limit the max number of gRPC connections by default, acquiring a connection could suspend a thread if there is no available connection.

gRPC calls are scheduled to a dedicated background thread pool. Workers in the thread pool are responsible to acquire the connection before starting the RPC call.

There could be a race condition that a worker thread handles some gRPC calls and then switches to a new call which will acquire new connections. If the number of connections reaches the max, the worker thread is suspended and doesn't have a chance to switch to previous calls. The connections held by previous calls are, hence, never released.

This PR changes to not use blocking get when acquiring gRPC connections.

Fixes #14363.

Closes #14416.

PiperOrigin-RevId: 416282883
Bencodes pushed a commit to Bencodes/bazel that referenced this pull request Jan 10, 2022
With recent change to limit the max number of gRPC connections by default, acquiring a connection could suspend a thread if there is no available connection.

gRPC calls are scheduled to a dedicated background thread pool. Workers in the thread pool are responsible to acquire the connection before starting the RPC call.

There could be a race condition that a worker thread handles some gRPC calls and then switches to a new call which will acquire new connections. If the number of connections reaches the max, the worker thread is suspended and doesn't have a chance to switch to previous calls. The connections held by previous calls are, hence, never released.

This PR changes to not use blocking get when acquiring gRPC connections.

Fixes bazelbuild#14363.

Closes bazelbuild#14416.

PiperOrigin-RevId: 416282883
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

Successfully merging this pull request may close these issues.

bazel 5.0.0rc2 hangs when both --bes_backend and --build_event_binary_file specified
2 participants