Skip to content

fix: Improve server stop handling with graceful shutdowns #3525

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

Merged
merged 2 commits into from
Jan 27, 2025

Conversation

lftk
Copy link
Contributor

@lftk lftk commented Jan 24, 2025

参考 gRPC 官方示例,当 Server 未能及时优雅退出时,执行强制退出。

https://github.com/grpc/grpc-go/blob/2fd426d0919db7e9e7920e365dd30674c4c9fe90/examples/features/gracefulstop/server/main.go#L89-L100

@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Jan 24, 2025
@lftk lftk force-pushed the fix/graceful-stop branch 2 times, most recently from fe744f5 to f2eda53 Compare January 24, 2025 09:13
@shenqidebaozi
Copy link
Member

shenqidebaozi commented Jan 24, 2025

This pull request includes several changes to improve the handling of server shutdowns and enhance test coverage for both gRPC and HTTP servers. The most important changes include modifications to the server shutdown logic, the addition of new tests, and synchronization improvements in the test code.

Improvements to server shutdown logic:

  • transport/grpc/server.go: Modified the Stop method to perform a force stop if the server cannot stop gracefully within the given context timeout.
  • transport/http/server.go: Updated the Stop method to log a warning and perform a force stop if the server cannot shut down gracefully within the context timeout.

Addition of new tests:

Synchronization improvements in test code:

These changes aim to enhance the robustness of server shutdown processes and improve the reliability of tests.

@lftk lftk force-pushed the fix/graceful-stop branch from 6923897 to 8fcdefc Compare January 25, 2025 05:27
- Remove default stop timeout
- Add context handling for server stop
- Implement graceful stop for gRPC server
- Enhance HTTP server shutdown logic
- Use shared context for server operations
@lftk lftk force-pushed the fix/graceful-stop branch from 8fcdefc to 54d2c99 Compare January 25, 2025 05:34
shenqidebaozi
shenqidebaozi previously approved these changes Jan 26, 2025
Copy link
Member

@shenqidebaozi shenqidebaozi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dosubot dosubot bot added the LGTM label Jan 26, 2025
@shenqidebaozi shenqidebaozi merged commit 2097002 into go-kratos:main Jan 27, 2025
33 checks passed
chaosue pushed a commit to chaosue/kratos that referenced this pull request Mar 12, 2025
…3525)

* Improve server stop handling with graceful shutdowns

- Remove default stop timeout
- Add context handling for server stop
- Implement graceful stop for gRPC server
- Enhance HTTP server shutdown logic
- Use shared context for server operations

* Remove unnecessary error logging in gRPC server test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LGTM size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants