[AN] Remove readiness blocking on `gracefulDone` channel by m-Peter · Pull Request #8664 · onflow/flow-go

m-Peter · 2026-08-23T14:22:54Z

Follow-up on #8662 .

On #8662 (comment), I suggested that <-gracefulDone is better to be moved outside the select, but after carefully revisiting the unit tests, it should in fact be removed entirely, as it can still block after the timeout.

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

Summary by CodeRabbit

Bug Fixes
- Improved server shutdown behavior when active streams exceed the graceful shutdown timeout.
- Prevented shutdown from waiting unnecessarily after forced termination.
- Reduced delays when terminating servers with streams that do not complete promptly.
- Improved cancellation handling for delayed streaming operations during shutdown.
Tests
- Expanded shutdown coverage for delayed active streams, interceptor cancellation, and immediate shutdown with no active streams.

github-actions · 2026-08-23T14:23:07Z

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

coderabbitai · 2026-08-23T14:24:16Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 30098368-4160-4684-94c3-60c7591f5108

📥 Commits

Reviewing files that changed from the base of the PR and between 54cdafe and 9dec934.

📒 Files selected for processing (1)

module/grpcserver/server_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

module/grpcserver/server_test.go

Included review availability: Your plan provides up to 4 included reviews per hour; 2 remain after this review.

📝 Walkthrough

Walkthrough

The shutdown worker returns after g.server.Stop() on the timeout path. Streaming shutdown tests add configurable handler delays for active-stream, interceptor-cancellation, and no-active-stream scenarios.

Changes

gRPC shutdown behavior

Layer / File(s)	Summary
Force-stop shutdown and test coverage `module/grpcserver/server.go`, `module/grpcserver/server_test.go`	The timeout path no longer waits for the graceful-stop goroutine after force-stopping the server. Test streaming handlers support blocking delays, and shutdown tests configure delays for active-stream, interceptor, and no-active-stream scenarios.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 9dec9

This localized change removes a shutdown wait that can block past its timeout; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: vishalchangrani

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 files.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: removing the blocking wait on the `gracefulDone` channel during graceful shutdown.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch mpeter/access-node-graceful-shutdown

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

m-Peter · 2026-08-23T14:24:41Z


 	rawServer := grpc.NewServer()
-	handler := &blockingStreamServer{started: make(chan struct{})}
+	handler := &blockingStreamServer{


By adding <-gracefulDone as the final statement on shutdownWorker, this test (TestGrpcServerShutdown_WithActiveStream), fails with:

could not close done channel on time: failed to shutdown all components on time

showcasing the blocking that was still possible.

codecov-commenter · 2026-08-23T14:26:26Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

m-Peter · 2026-08-23T14:42:14Z

 func (s *blockingStreamServer) Stream(stream grpc.ServerStream) error {
 	close(s.started)
+	if s.blockDuration > 0 {
+		time.Sleep(s.blockDuration)


Besides blocking on the done channel readiness, also add some artificial blocking with time.Sleep, whenever configured.

Co-authored-by: Leo Zhang <zhangchiqing@gmail.com>

* fix the graceful shutdown by adding a timeout * make tidy * cancel streaming RPCs on shutdown via interceptor Adds a StreamServerInterceptor that couples stream.Context() to the node's SignalerContext, so long-lived subscriptions exit cleanly when shutdown begins. GracefulStop then completes without needing the force-stop fallback in the common case. The 5s timeout remains as a backstop for any hung RPCs. Also defaults NewGrpcServer's timeout when zero, switches the shutdown timer to time.NewTimer to avoid a leak on the fast path, and brackets type references in godocs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * drop unnecessary godoc and whitespace changes Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * address review comments * Remove entirely blocking on gracefulDone channel readiness * Update comment regarding time.Sleep on Stream() Co-authored-by: Leo Zhang <zhangchiqing@gmail.com> * Fix linting issue around comment --------- Co-authored-by: Leo Zhang (zhangchiqing) <zhangchiqing@gmail.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Ardit Marku <markoupetr@gmail.com>

Remove entirely blocking on gracefulDone channel readiness

4f48c22

m-Peter self-assigned this Aug 23, 2026

m-Peter requested a review from a team as a code owner August 23, 2026 14:22

m-Peter added the Bugfix label Aug 23, 2026

m-Peter commented Aug 23, 2026

View reviewed changes

m-Peter requested review from vishalchangrani and zhangchiqing August 23, 2026 14:24

m-Peter commented Aug 23, 2026

View reviewed changes

zhangchiqing approved these changes Aug 25, 2026

View reviewed changes

Comment thread module/grpcserver/server_test.go

m-Peter and others added 2 commits August 26, 2026 09:17

Update comment regarding time.Sleep on Stream()

54cdafe

Co-authored-by: Leo Zhang <zhangchiqing@gmail.com>

Fix linting issue around comment

9dec934

m-Peter changed the title ~~[AN] Remove entirely readiness blocking on gracefulDone channel~~ [AN] Remove readiness blocking on gracefulDone channel Aug 26, 2026

vishalchangrani approved these changes Aug 27, 2026

View reviewed changes

vishalchangrani added this pull request to the merge queue Aug 27, 2026

Merged via the queue into master with commit 7b1c658 Aug 27, 2026
62 checks passed

vishalchangrani deleted the mpeter/access-node-graceful-shutdown branch August 27, 2026 21:19

vishalchangrani mentioned this pull request Aug 27, 2026

[v0.51] [AN] Backport graceful shutdown fix (#8662, #8664) #8676

Merged

Navigation Menu

Uh oh!

[AN] Remove readiness blocking on `gracefulDone` channel - #8664

[AN] Remove readiness blocking on `gracefulDone` channel#8664
vishalchangrani merged 3 commits into
masterfrom
mpeter/access-node-graceful-shutdown

m-Peter commented Aug 23, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

github-actions Bot commented Aug 23, 2026

Uh oh!

coderabbitai Bot commented Aug 23, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

m-Peter Aug 23, 2026

Uh oh!

codecov-commenter commented Aug 23, 2026

Uh oh!

m-Peter Aug 23, 2026

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

m-Peter commented Aug 23, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

github-actions Bot commented Aug 23, 2026

Dependency Review

Scanned Files

Uh oh!

coderabbitai Bot commented Aug 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

m-Peter Aug 23, 2026

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Aug 23, 2026

Codecov Report

Uh oh!

m-Peter Aug 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

m-Peter commented Aug 23, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Aug 23, 2026 •

edited

Loading