From e6fc10faf698d2ad7eba97d4c9f5b8a1b505da64 Mon Sep 17 00:00:00 2001 From: Zaki Manian Date: Sat, 17 Nov 2018 00:10:22 -0800 Subject: [PATCH] R4R: Add timeouts to http servers (#2780) * Replaces our current http servers where connections stay open forever with ones with timeouts to prevent file descriptor exhaustion * Use the correct handler * Put in go routines * fix err * changelog * rpc: export Read/WriteTimeout The `broadcast_tx_commit` endpoint has it's own timeout. If this is longer than the http server's WriteTimeout, the user will receive an error. Here, we export the WriteTimeout and set the broadcast_tx_commit timeout to be less than it. In the future, we should use a config struct for the timeouts to avoid the need to export. The broadcast_tx_commit timeout may also become configurable, but we must check that it's less than the server's WriteTimeout. --- CHANGELOG_PENDING.md | 1 + rpc/core/mempool.go | 4 +++- rpc/core/pipe.go | 5 ++--- rpc/lib/server/http_server.go | 38 +++++++++++++++++++++++++---------- 4 files changed, 33 insertions(+), 15 deletions(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 7c7e07175..f49ddd725 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -34,6 +34,7 @@ program](https://hackerone.com/tendermint). ### IMPROVEMENTS: +- [rpc] \#2780 Add read and write timeouts to HTTP servers - [state] \#2765 Make "Update to validators" msg value pretty (@danil-lashin) - [p2p] \#2857 "Send failed" is logged at debug level instead of error. diff --git a/rpc/core/mempool.go b/rpc/core/mempool.go index 598774921..7b3c368af 100644 --- a/rpc/core/mempool.go +++ b/rpc/core/mempool.go @@ -9,6 +9,7 @@ import ( abci "github.com/tendermint/tendermint/abci/types" ctypes "github.com/tendermint/tendermint/rpc/core/types" + rpcserver "github.com/tendermint/tendermint/rpc/lib/server" "github.com/tendermint/tendermint/types" ) @@ -194,7 +195,8 @@ func BroadcastTxCommit(tx types.Tx) (*ctypes.ResultBroadcastTxCommit, error) { } // Wait for the tx to be included in a block or timeout. - var deliverTxTimeout = 10 * time.Second // TODO: configurable? + // TODO: configurable? + var deliverTxTimeout = rpcserver.WriteTimeout / 2 select { case deliverTxResMsg := <-deliverTxResCh: // The tx was included in a block. deliverTxRes := deliverTxResMsg.(types.EventDataTx) diff --git a/rpc/core/pipe.go b/rpc/core/pipe.go index 188ea1c36..ae8ae056a 100644 --- a/rpc/core/pipe.go +++ b/rpc/core/pipe.go @@ -1,8 +1,6 @@ package core import ( - "time" - "github.com/tendermint/tendermint/consensus" crypto "github.com/tendermint/tendermint/crypto" dbm "github.com/tendermint/tendermint/libs/db" @@ -10,6 +8,7 @@ import ( mempl "github.com/tendermint/tendermint/mempool" "github.com/tendermint/tendermint/p2p" "github.com/tendermint/tendermint/proxy" + rpcserver "github.com/tendermint/tendermint/rpc/lib/server" sm "github.com/tendermint/tendermint/state" "github.com/tendermint/tendermint/state/txindex" "github.com/tendermint/tendermint/types" @@ -21,7 +20,7 @@ const ( maxPerPage = 100 ) -var subscribeTimeout = 5 * time.Second +var subscribeTimeout = rpcserver.WriteTimeout / 2 //---------------------------------------------- // These interfaces are used by RPC and must be thread safe diff --git a/rpc/lib/server/http_server.go b/rpc/lib/server/http_server.go index a5f8692f9..1fd422a9b 100644 --- a/rpc/lib/server/http_server.go +++ b/rpc/lib/server/http_server.go @@ -27,6 +27,17 @@ const ( // maxBodyBytes controls the maximum number of bytes the // server will read parsing the request body. maxBodyBytes = int64(1000000) // 1MB + + // same as the net/http default + maxHeaderBytes = 1 << 20 + + // Timeouts for reading/writing to the http connection. + // Public so handlers can read them - + // /broadcast_tx_commit has it's own timeout, which should + // be less than the WriteTimeout here. + // TODO: use a config instead. + ReadTimeout = 3 * time.Second + WriteTimeout = 20 * time.Second ) // StartHTTPServer takes a listener and starts an HTTP server with the given handler. @@ -34,10 +45,13 @@ const ( // NOTE: This function blocks - you may want to call it in a go-routine. func StartHTTPServer(listener net.Listener, handler http.Handler, logger log.Logger) error { logger.Info(fmt.Sprintf("Starting RPC HTTP server on %s", listener.Addr())) - err := http.Serve( - listener, - RecoverAndLogHandler(maxBytesHandler{h: handler, n: maxBodyBytes}, logger), - ) + s := &http.Server{ + Handler: RecoverAndLogHandler(maxBytesHandler{h: handler, n: maxBodyBytes}, logger), + ReadTimeout: ReadTimeout, + WriteTimeout: WriteTimeout, + MaxHeaderBytes: maxHeaderBytes, + } + err := s.Serve(listener) logger.Info("RPC HTTP server stopped", "err", err) return err } @@ -53,13 +67,15 @@ func StartHTTPAndTLSServer( ) error { logger.Info(fmt.Sprintf("Starting RPC HTTPS server on %s (cert: %q, key: %q)", listener.Addr(), certFile, keyFile)) - err := http.ServeTLS( - listener, - RecoverAndLogHandler(maxBytesHandler{h: handler, n: maxBodyBytes}, logger), - certFile, - keyFile, - ) - logger.Info("RPC HTTPS server stopped", "err", err) + s := &http.Server{ + Handler: RecoverAndLogHandler(maxBytesHandler{h: handler, n: maxBodyBytes}, logger), + ReadTimeout: ReadTimeout, + WriteTimeout: WriteTimeout, + MaxHeaderBytes: maxHeaderBytes, + } + err := s.ServeTLS(listener, certFile, keyFile) + + logger.Error("RPC HTTPS server stopped", "err", err) return err }