fix(server): only shutdown gRPC if HTTP server is absent #18
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Update Shutdown logic to only gracefully stop the gRPC server if the HTTP server is not present. This prevents unintended shutdown behavior when both servers are running.
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a shutdown race condition issue by modifying the gRPC server shutdown logic to only call GracefulStop() when no HTTP server is present. This prevents interference between HTTP and gRPC shutdown processes in mixed server environments like Cloud Run.
Key changes:
- Added conditional check to prevent gRPC shutdown when HTTP server exists
- Ensures proper graceful shutdown coordination in mixed HTTP/gRPC setups
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR modifies our shutdown logic to only call gRPC.GracefulStop() when an HTTP server is not enabled.
Why is this change needed?
When we run our service on Cloud Run, we primarily use the HTTP server to handle incoming requests, and gRPC traffic is served over the same port. In this mixed HTTP/gRPC setup, the HTTP server is responsible for managing the graceful shutdown of the entire application.
Calling gRPC.GracefulStop() in this scenario can interfere with the HTTP server's shutdown process, potentially causing premature connection termination or race conditions. The gRPC server should be treated as a "guest" that is shut down automatically when the main HTTP server stops listening.
This approach is based on the recommended practice for shutting down servers that handle both gRPC and other traffic on the same port. For more technical details, see this explanation: grpc/grpc-go#1384 (comment).
This change ensures a more stable and predictable graceful shutdown, especially in our Cloud Run environment.