From 6489bec79e6baea90985a7735716d673ab84dc79 Mon Sep 17 00:00:00 2001 From: Eugene Aleynikov Date: Tue, 20 May 2025 17:17:13 -0700 Subject: [PATCH 1/2] Implemented health probe for cleaner shutdown --- node/node.go | 6 ++++++ node/rpcstack.go | 45 +++++++++++++++++++++++++++++++++++++++------ 2 files changed, 45 insertions(+), 6 deletions(-) diff --git a/node/node.go b/node/node.go index ec7382e725..88246c8e1b 100644 --- a/node/node.go +++ b/node/node.go @@ -519,6 +519,12 @@ func (n *Node) stopRPC() { n.ws.stop() n.httpAuth.stop() n.wsAuth.stop() + + n.http.wait() + n.ws.wait() + n.httpAuth.wait() + n.wsAuth.wait() + n.ipc.stop() n.stopInProc() } diff --git a/node/rpcstack.go b/node/rpcstack.go index 6d3828ec2b..39997a6164 100644 --- a/node/rpcstack.go +++ b/node/rpcstack.go @@ -70,12 +70,13 @@ type httpServer struct { timeouts rpc.HTTPTimeouts mux http.ServeMux // registered handlers go here - mu sync.Mutex - server *http.Server - listener net.Listener // non-nil when server is running + mu sync.Mutex + server *http.Server + listener net.Listener // non-nil when server is running + ready bool + shutdownWG sync.WaitGroup // WG to wait for shutdown // HTTP RPC handler things. - httpConfig httpConfig httpHandler atomic.Value // *rpcHandler @@ -93,6 +94,9 @@ type httpServer struct { const ( shutdownTimeout = 5 * time.Second + // give pending requests stopPendingRequestTimeout the time to finish when the server is stopped + // if readiness probe period is 5 seconds, this is enough time for health check to be triggered + stopPendingRequestTimeout = 7 * time.Second ) func newHTTPServer(log log.Logger, timeouts rpc.HTTPTimeouts) *httpServer { @@ -167,6 +171,7 @@ func (h *httpServer) start() error { } h.log.Info("WebSocket enabled", "url", url) } + h.ready = true // if server is websocket only, return after logging if !h.rpcAllowed() { return nil @@ -197,6 +202,22 @@ func (h *httpServer) start() error { } func (h *httpServer) ServeHTTP(w http.ResponseWriter, r *http.Request) { + // server health probe endpoints + if r.Method == http.MethodGet { + // readiness probe fails during shutdown + if r.URL.Path == "/readyz" { + if h.ready { + w.WriteHeader(http.StatusNoContent) + } else { + w.WriteHeader(http.StatusServiceUnavailable) + } + return + // liveness probe always succeeds + } else if r.URL.Path == "/livez" { + w.WriteHeader(http.StatusNoContent) + return + } + } // check if ws request and serve if ws enabled ws := h.wsHandler.Load().(*rpcHandler) if ws != nil && isWebsocket(r) { @@ -257,8 +278,20 @@ func validatePrefix(what, path string) error { // stop shuts down the HTTP server. func (h *httpServer) stop() { h.mu.Lock() - defer h.mu.Unlock() - h.doStop() + // unit test executes stop multiple times, so we cannot increment the WG in the start method + h.shutdownWG = sync.WaitGroup{} + h.shutdownWG.Add(1) + h.ready = false + time.AfterFunc(stopPendingRequestTimeout, func() { + defer h.mu.Unlock() + h.doStop() + h.shutdownWG.Done() + }) +} + +// wait waits for the server to shutdown. +func (h *httpServer) wait() { + h.shutdownWG.Wait() } func (h *httpServer) doStop() { From 8b06db6722eceda4328aa8e998adb6bcfe520dde Mon Sep 17 00:00:00 2001 From: Eugene Aleynikov Date: Thu, 12 Jun 2025 12:57:59 -0700 Subject: [PATCH 2/2] Make shutdown delay to be conditional and off by default --- cmd/geth/main.go | 1 + cmd/utils/flags.go | 10 ++++++++++ node/config.go | 4 ++++ node/defaults.go | 2 ++ node/node.go | 8 ++++---- node/rpcstack.go | 20 +++++++++----------- node/rpcstack_test.go | 2 +- 7 files changed, 31 insertions(+), 16 deletions(-) diff --git a/cmd/geth/main.go b/cmd/geth/main.go index 289af90828..da9b30a0f2 100644 --- a/cmd/geth/main.go +++ b/cmd/geth/main.go @@ -183,6 +183,7 @@ var ( utils.AllowUnprotectedTxs, utils.BatchRequestLimit, utils.BatchResponseMaxSize, + utils.ShutdownDelay, } metricsFlags = []cli.Flag{ diff --git a/cmd/utils/flags.go b/cmd/utils/flags.go index eadc6e8b2d..cffcccef06 100644 --- a/cmd/utils/flags.go +++ b/cmd/utils/flags.go @@ -760,6 +760,12 @@ var ( Value: node.DefaultConfig.BatchResponseMaxSize, Category: flags.APICategory, } + ShutdownDelay = &cli.DurationFlag{ + Name: "shutdown-delay", + Usage: "Sets a delay between receiving SIGTERM and shutting down the node", + Value: node.DefaultConfig.ShutdownDelay, + Category: flags.APICategory, + } // Network Settings MaxPeersFlag = &cli.IntFlag{ @@ -1204,6 +1210,10 @@ func setHTTP(ctx *cli.Context, cfg *node.Config) { if ctx.IsSet(BatchResponseMaxSize.Name) { cfg.BatchResponseMaxSize = ctx.Int(BatchResponseMaxSize.Name) } + + if ctx.IsSet(ShutdownDelay.Name) { + cfg.ShutdownDelay = ctx.Duration(ShutdownDelay.Name) + } } // setGraphQL creates the GraphQL listener interface string from the set diff --git a/node/config.go b/node/config.go index dc436876cc..96acca6b6a 100644 --- a/node/config.go +++ b/node/config.go @@ -24,6 +24,7 @@ import ( "path/filepath" "runtime" "strings" + "time" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/crypto" @@ -211,6 +212,9 @@ type Config struct { EnablePersonal bool `toml:"-"` DBEngine string `toml:",omitempty"` + + // ShutdownDelay is the delay between receiving SIGTERM and shutting down the node. + ShutdownDelay time.Duration `toml:",omitempty"` } // IPCEndpoint resolves an IPC endpoint based on a configured value, taking into diff --git a/node/defaults.go b/node/defaults.go index 307d9e186a..3c2ac49adc 100644 --- a/node/defaults.go +++ b/node/defaults.go @@ -21,6 +21,7 @@ import ( "os/user" "path/filepath" "runtime" + "time" "github.com/ethereum/go-ethereum/p2p" "github.com/ethereum/go-ethereum/p2p/nat" @@ -73,6 +74,7 @@ var DefaultConfig = Config{ NAT: nat.Any(), }, DBEngine: "", // Use whatever exists, will default to Pebble if non-existent and supported + ShutdownDelay: 0 * time.Second, } // DefaultDataDir is the default data directory to use for the databases and other diff --git a/node/node.go b/node/node.go index ccb20efbe7..e256624d6f 100644 --- a/node/node.go +++ b/node/node.go @@ -150,10 +150,10 @@ func New(conf *Config) (*Node, error) { } // Configure RPC servers. - node.http = newHTTPServer(node.log, conf.HTTPTimeouts) - node.httpAuth = newHTTPServer(node.log, conf.HTTPTimeouts) - node.ws = newHTTPServer(node.log, rpc.DefaultHTTPTimeouts) - node.wsAuth = newHTTPServer(node.log, rpc.DefaultHTTPTimeouts) + node.http = newHTTPServer(node.log, conf.HTTPTimeouts, conf.ShutdownDelay) + node.httpAuth = newHTTPServer(node.log, conf.HTTPTimeouts, conf.ShutdownDelay) + node.ws = newHTTPServer(node.log, rpc.DefaultHTTPTimeouts, conf.ShutdownDelay) + node.wsAuth = newHTTPServer(node.log, rpc.DefaultHTTPTimeouts, conf.ShutdownDelay) node.ipc = newIPCServer(node.log, conf.IPCEndpoint()) return node, nil diff --git a/node/rpcstack.go b/node/rpcstack.go index 39997a6164..1669901e6e 100644 --- a/node/rpcstack.go +++ b/node/rpcstack.go @@ -70,11 +70,12 @@ type httpServer struct { timeouts rpc.HTTPTimeouts mux http.ServeMux // registered handlers go here - mu sync.Mutex - server *http.Server - listener net.Listener // non-nil when server is running - ready bool - shutdownWG sync.WaitGroup // WG to wait for shutdown + mu sync.Mutex + server *http.Server + listener net.Listener // non-nil when server is running + ready bool + shutdownWG sync.WaitGroup // WG to wait for shutdown + shutdownDelay time.Duration // HTTP RPC handler things. httpConfig httpConfig @@ -94,13 +95,10 @@ type httpServer struct { const ( shutdownTimeout = 5 * time.Second - // give pending requests stopPendingRequestTimeout the time to finish when the server is stopped - // if readiness probe period is 5 seconds, this is enough time for health check to be triggered - stopPendingRequestTimeout = 7 * time.Second ) -func newHTTPServer(log log.Logger, timeouts rpc.HTTPTimeouts) *httpServer { - h := &httpServer{log: log, timeouts: timeouts, handlerNames: make(map[string]string)} +func newHTTPServer(log log.Logger, timeouts rpc.HTTPTimeouts, shutdownDelay time.Duration) *httpServer { + h := &httpServer{log: log, timeouts: timeouts, handlerNames: make(map[string]string), shutdownDelay: shutdownDelay} h.httpHandler.Store((*rpcHandler)(nil)) h.wsHandler.Store((*rpcHandler)(nil)) @@ -282,7 +280,7 @@ func (h *httpServer) stop() { h.shutdownWG = sync.WaitGroup{} h.shutdownWG.Add(1) h.ready = false - time.AfterFunc(stopPendingRequestTimeout, func() { + time.AfterFunc(h.shutdownDelay, func() { defer h.mu.Unlock() h.doStop() h.shutdownWG.Done() diff --git a/node/rpcstack_test.go b/node/rpcstack_test.go index 54e58cccb2..e6e663a37f 100644 --- a/node/rpcstack_test.go +++ b/node/rpcstack_test.go @@ -241,7 +241,7 @@ func createAndStartServer(t *testing.T, conf *httpConfig, ws bool, wsConf *wsCon if timeouts == nil { timeouts = &rpc.DefaultHTTPTimeouts } - srv := newHTTPServer(testlog.Logger(t, log.LvlDebug), *timeouts) + srv := newHTTPServer(testlog.Logger(t, log.LvlDebug), *timeouts, 0*time.Second) assert.NoError(t, srv.enableRPC(apis(), *conf)) if ws { assert.NoError(t, srv.enableWS(nil, *wsConf))