Add graceful shutdown of RHP4 server with defer rhp4.Close()#944
Add graceful shutdown of RHP4 server with defer rhp4.Close()#944ChrisSchinnerl merged 13 commits intomasterfrom
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review.
Tip: disable this comment in your organization's Code Review settings.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
The TestRPCFreeSectors test is failing, and it seems unrelated to these changes. The test is NDF. Taking a look at it now. |
ChrisSchinnerl
left a comment
ChrisSchinnerl
left a comment
There was a problem hiding this comment.
go.mod will require an update after merging the coreutils PR to a commit that actually exists on master.
Co-authored-by: Christopher Schinnerl <3903476+ChrisSchinnerl@users.noreply.github.com>
cmd/hostd/run.go
Outdated
| log.Error("failed to stop listener", zap.Error(err)) | ||
| } | ||
| } | ||
| rhp4.Close() |
There was a problem hiding this comment.
This is still running after stopListenerFuncs. Should be called before the loop.
There was a problem hiding this comment.
If you rename stopListenerFuncs to closeFns and have rhp4.Close be the first you add it might be slightly cleaner.
cmd/hostd/run.go
Outdated
| log.Error("failed to stop listener", zap.Error(err)) | ||
| } | ||
| } | ||
| rhp4.Close() |
There was a problem hiding this comment.
If you rename stopListenerFuncs to closeFns and have rhp4.Close be the first you add it might be slightly cleaner.
There was a problem hiding this comment.
Pull request overview
Adds RHP4 server shutdown hooks to prevent lingering goroutines/resources, and adjusts integration tests to reflect updated client-side behavior.
Changes:
- Ensure RHP4 server instances are closed during shutdown/cleanup (hostd run path + integration tests).
- Fix
TestRPCFreeSectorsexpected-root computation by matching the client’s index sorting behavior. - Bump
go.sia.tech/core/go.sia.tech/coreutilsdependency versions and add a changeset entry.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
cmd/hostd/run.go |
Adds RHP4 server close to shutdown path and refactors shutdown closure handling. |
internal/integration/rhp/v4/rhp4_test.go |
Adds cleanup for test RHP4 servers; updates FreeSectors expectation logic. |
go.mod |
Updates core and coreutils versions required for the behavior change. |
go.sum |
Syncs module checksums with updated dependencies. |
.changeset/graceful_shutdown_of_rhp4_server_via_defer_close.md |
Documents the change via a new changeset entry. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Fixes SiaFoundation/coreutils#412
Relies on SiaFoundation/coreutils#413
Must edit go.mod once coreutils and core have tagged releases for these changes.