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

Is graceful_exit actually graceful? #148

Open
jalaziz opened this issue Jan 11, 2022 · 1 comment
Open

Is graceful_exit actually graceful? #148

jalaziz opened this issue Jan 11, 2022 · 1 comment

Comments

@jalaziz
Copy link

jalaziz commented Jan 11, 2022

graceful_exit ultimate calls server.close() which is defined as:

    def close(self) -> None:
        """Stops accepting new connections, cancels all currently running
        requests. Request handlers are able to handle `CancelledError` and
        exit properly.
        """
        if self._server is None:
            raise RuntimeError('Server is not started')
        self._server.close()
        for handler in self._handlers:
            handler.close()

This suggests graceful_exit is not as graceful as it could be because it ultimately cancels inflight requests. Wouldn't a more graceful approach be to simply call self._server.close() and then rely on server.wait_closed() to wait until all request handles end properly?

Or am I mistaken with the behavior of handler.close(). Digging into the code, it also seems that handler.wait_closed() only waits for handler completion when handler.close() was called, otherwise it returns immediately.

Assuming I'm interpreting the code correctly, is there any way to achieve a graceful exit that allows in flight requests to run to completion? I do realize this is a slippery slope as gRPC streaming can complicate the matter, but for that case, it would be great to give connections a fixed deadline to end on their own after which a forced cancellation is triggered.

I'd be happy to contribute a fix here, but I was hoping to get a better understanding of why the current implementation cancels in flight requests and anything else I'm missing.

@vmagamedov
Copy link
Owner

The main reason grpclib don't have a very graceful exit is because h2 library don't implement graceful connection close. Ideal solution would be to send GOAWAY frame from server to clients when we call server.close(). This should allow you to finish current requests without accepting new requests. But currently this renders HTTP/2 connection useless (closed), and your current requests can't finish correctly:

https://github.com/python-hyper/h2/blob/53feb0e0d8ddd28fa2dbd534d519a8eefd441f14/src/h2/connection.py#L207-L208

So we currently don't have abilities to inform client not to send new requests (over already established connections).

Currently to implement graceful exit you may need help from your infrastructure, you can have a HTTP/2-capable load balancer which manages incoming traffic for your gRPC servers. When you want to remove a server from serving requests you mark your server as "unhealthy" or "terminating" and load balancer stops sending new requests to your server. Then after some timeout you can stop your server.

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

2 participants