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
grpc: Wait until resources finish cleaning up in Stop() and GracefulStop() #6489
Commits on Jul 31, 2023
-
transport/http2server: wait in Close() until loopyWriter terminated
When Server.Stop() returns it is not guaranteed that the loopyWriter go-routine terminated. This can lead to a panic or a testing.(*common).logDepth() race condition in Go Tests because t.Log is used after or during the testcase terminates. This can happen when: - a GRPC server is started in a Go test, - the GRPC logs are forwarded to t.Log, - loopyWriter.run logs an error after server.Stop() and the Test method returns. grpc@v1.57.0/internal/leakcheck is unable to detect that the loopyWriter go-routine continues to run after server.Stop() because it waits up to 10s for go-routines to terminate after a test finishes. The loopyWriter returns much faster after Stop() returns. To make server.Stop() wait until loopyWriter terminated: - rename the existing writerDone field, which is only used in tests, to loopyWriterDone, the writerDone channel is closed when the loopyWriter go-routine exited - change http2server.Close to wait until loopyWriterDone is closed
Commits on Aug 9, 2023
-
Revert "transport/http2server: wait in Close() until loopyWriter term…
…inated" This reverts commit ad96f3c.
-
server: Wait in GracefulStop and Stop until loopyWriter goroutine exits
It was not guaranteed that when Stop() and GracefulStop() returned, the loopyWriter go-routines terminated. This can lead to a panic or a testing.(*common).logDepth() race condition in Go Tests because t.Log is used after or during the testcase terminates. This can happen when: - a GRPC server is started in a Go test, - the GRPC logs are forwarded to t.Log, - loopyWriter.run logs an error after server.Stop() and the Test method returns. grpc@v1.57.0/internal/leakcheck is unable to detect that the loopyWriter go-routine continues to run after server.Stop() because it waits up to 10s for go-routines to terminate after a test finishes. The loopyWriter returns much faster after Stop() returns. To make ensure Stop and GracefulStop only return after the loopyWriter go-routine terminated: - rename http2Server.writerDone to loopyWriterDone, the channel was only used in test and is closed when loopyWriter returns - wait in http2Server.HandleStreams() until loopyWriterDone was closed - wait in Server.Stop() until all connections were removed, this is already done in GracefulStop. To abort GracefulStop() when Stop() is called a local copy of Server.lis and Server.conns was created and both fields were set to nil and then s.csv.Broadcast() was called to wake up an eventually sleeping GracefulStop() execution. This does not work anymore when we need to wait for connections to be removed in Stop() we need to check the length of Server.conns. Instead a new abortGracefulStop atomicBool is introduced, that is set to true to indicate that a GracefulStop() invocation should be aborted when Stop() is run.
Commits on Aug 11, 2023
Commits on Aug 28, 2023
-
-
-
-
-
server: stopServerWorkers also in GracefulStop
The call was only done in Stop() and missing in GracefulStop()
-
server: consolidate shared Stop and GracefulStop code
Move code that is used in Stop and GracefulStop to helper functions instead of duplicating it. This makes it easier to compare Stop() and GracefulStop.
Commits on Sep 7, 2023
Commits on Oct 20, 2023
-
Revert "server: close listeners and wait for serveWG threads before d…
…raining" This reverts commit 0264c98ecd860431ac82a4e799cf6282e08e8a04.
Commits on Oct 26, 2023
-
- drain transporters after waiting serve handlers to terminate - call closeServerTransports only on non graceful shutdowns, to abruptly close the connections
-
tests: add server.Stop() and server.GracefulStop testcases
Add 2 testcases that ensure that: - GracefulStop blocks and waits until ongoing client grpc calls finis, - Stop() returns despite ongoing blocking grpc calls and clients receive a connection error
-
Delete internal/transport/http2_server.go.orig
The file was added accidentally