Skip to content

cmd/soroban-rpc: Shutdown HTTP server gracefully using server.Shutdown instead of supporthttp Package - #566

Merged
stellarsaur merged 8 commits into
mainfrom
soroban-http-graceful-shutdown
Apr 10, 2023
Merged

cmd/soroban-rpc: Shutdown HTTP server gracefully using server.Shutdown instead of supporthttp Package#566
stellarsaur merged 8 commits into
mainfrom
soroban-http-graceful-shutdown

Conversation

@stellarsaur

Copy link
Copy Markdown
Contributor

What

Currently, we rely on the supporthttp package to finish pending HTTP requests before shutting down. This PR removes that dependency and uses the server.Shutdown method directly, alongside the appropriate OS signal handling.

This PR still uses the same values as before for relevant timeouts:

  • default read timeout still 5s
  • shutdown grace period still 10s (supporthttp default)

Open to suggestions on testing this, or if it's even necessary.

Closes #499

Why

Without graceful shutdown, when you kill soroban-rpc it may drop ongoing HTTP requests. We would like to wait a grace period while finishing any outgoing requests and closing any open connections. Additionally, we don't want to rely on external packages for this (like how supporthttp relies on the Graceful package).

@stellarsaur
stellarsaur requested a review from a team April 6, 2023 23:11
@stellarsaur stellarsaur added the enhancement New feature or request label Apr 6, 2023
Comment thread cmd/soroban-rpc/internal/daemon/daemon.go Outdated
Comment thread cmd/soroban-rpc/internal/daemon/daemon.go Outdated

@tsachiherman tsachiherman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I like this implementation. I think that it could be further improve shutting down all the other services by passing the context to d.Close().

What I had in mind is a "hard limit" of terminating the service within 5 second or letting it shut down on its own faster. Keep in mind that if we won't persist data or disconnect existing connections - that's perfectly fine.

In case the shutdown takes more than 5 seconds, we should be generating a log entry telling us why this is the case ( i.e. one of the services is hanged.. )

Comment thread cmd/soroban-rpc/internal/daemon/daemon.go
stellarsaur and others added 3 commits April 10, 2023 14:37
Co-authored-by: Alfonso Acosta <alfonso@stellar.org>
Co-authored-by: Alfonso Acosta <alfonso@stellar.org>
@stellarsaur
stellarsaur merged commit 7545903 into main Apr 10, 2023
@stellarsaur
stellarsaur deleted the soroban-http-graceful-shutdown branch April 10, 2023 22:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

soroban-rpc: Graceful shutdown for HTTP server

4 participants