Fix #421. Add read timeout to API to make sure we don't keep thousands of connections open needlessly.

CHANGELOG.md

@@ -6,6 +6,7 @@
 - [Issue #416](https://github.com/influxdb/influxdb/issues/416). Improve the time it takes to drop database
 - [Issue #413](https://github.com/influxdb/influxdb/issues/413). Don't assume that group by interval is greater than a second
 - [Issue #415](https://github.com/influxdb/influxdb/issues/415). Include the database when sending an auth error back to the user
+- [Issue #421](https://github.com/influxdb/influxdb/issues/421). Make read timeout a config option
 
 ### Bugfixes
 

config.toml.sample

@@ -23,6 +23,11 @@ port     = 8086    # binding is disabled if the port isn't set
 # ssl-port = 8084    # Ssl support is enabled if you set a port and cert
 # ssl-cert = /path/to/cert.pem
 
+# connections will timeout after this amount of time. Ensures that clients that misbehave 
+# and keep alive connections they don't use won't end up connection a million times.
+# However, if a request is taking longer than this to complete, could be a problem.
+read-timeout = "5s"
+
 [input_plugins]
 
   # Configure the graphite api

src/api/http/api.go

@@ -34,9 +34,10 @@ type HttpServer struct {
 	shutdown       chan bool
 	clusterConfig  *cluster.ClusterConfiguration
 	raftServer     *coordinator.RaftServer
+	readTimeout    time.Duration
 }
 
-func NewHttpServer(httpPort string, adminAssetsDir string, theCoordinator coordinator.Coordinator, userManager UserManager, clusterConfig *cluster.ClusterConfiguration, raftServer *coordinator.RaftServer) *HttpServer {
+func NewHttpServer(httpPort string, readTimeout time.Duration, adminAssetsDir string, theCoordinator coordinator.Coordinator, userManager UserManager, clusterConfig *cluster.ClusterConfiguration, raftServer *coordinator.RaftServer) *HttpServer {
 	self := &HttpServer{}
 	self.httpPort = httpPort
 	self.adminAssetsDir = adminAssetsDir
@@ -45,6 +46,7 @@ func NewHttpServer(httpPort string, adminAssetsDir string, theCoordinator coordi
 	self.shutdown = make(chan bool, 2)
 	self.clusterConfig = clusterConfig
 	self.raftServer = raftServer
+	self.readTimeout = readTimeout
 	return self
 }
 
@@ -146,9 +148,7 @@ func (self *HttpServer) Serve(listener net.Listener) {
 	}
 
 	go self.startSsl(p)
-	if err := libhttp.Serve(listener, p); err != nil && !strings.Contains(err.Error(), "closed network") {
-		panic(err)
-	}
+	self.serveListener(listener, p)
 }
 
 func (self *HttpServer) startSsl(p *pat.PatternServeMux) {
@@ -173,7 +173,12 @@ func (self *HttpServer) startSsl(p *pat.PatternServeMux) {
 		panic(err)
 	}
 
-	if err := libhttp.Serve(self.sslConn, p); err != nil && !strings.Contains(err.Error(), "closed network") {
+	self.serveListener(self.sslConn, p)
+}
+
+func (self *HttpServer) serveListener(listener net.Listener, p *pat.PatternServeMux) {
+	srv := &libhttp.Server{Handler: p, ReadTimeout: self.readTimeout}
+	if err := srv.Serve(listener); err != nil && !strings.Contains(err.Error(), "closed network") {
 		panic(err)
 	}
 }

src/api/http/api_test.go

@@ -9,6 +9,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"io/ioutil"
+	. "launchpad.net/gocheck"
 	"net"
 	libhttp "net/http"
 	"net/url"
@@ -16,7 +17,6 @@ import (
 	"protocol"
 	"testing"
 	"time"
-	. "launchpad.net/gocheck"
 )
 
 // Hook up gocheck into the gotest runner.
@@ -181,7 +181,7 @@ func (self *ApiSuite) SetUpSuite(c *C) {
 		dbUsers:       map[string]map[string]MockDbUser{"db1": map[string]MockDbUser{"db_user1": {Name: "db_user1", IsAdmin: false}}},
 	}
 	dir := c.MkDir()
-	self.server = NewHttpServer("", dir, self.coordinator, self.manager, nil, nil)
+	self.server = NewHttpServer("", 10*time.Second, dir, self.coordinator, self.manager, nil, nil)
 	var err error
 	self.listener, err = net.Listen("tcp4", ":8081")
 	c.Assert(err, IsNil)

src/configuration/config.toml

@@ -20,6 +20,11 @@ assets = "./admin"
 ssl-port = 8087    # Ssl support is enabled if you set a port and cert
 ssl-cert = "../cert.pem"
 
+# connections will timeout after this amount of time. Ensures that clients that misbehave 
+# and keep alive connections they don't use won't end up connection a million times.
+# However, if a request is taking longer than this to complete, could be a problem.
+read-timeout = "5s"
+
 [input_plugins]
 
   # Configure the graphite api

src/configuration/configuration.go

@@ -47,6 +47,9 @@ type duration struct {
 }
 
 func (d *duration) UnmarshalText(text []byte) error {
+	if len(text) == 0 {
+		return nil
+	}
 	var err error
 	d.Duration, err = time.ParseDuration(string(text))
 	return err
@@ -61,6 +64,7 @@ type ApiConfig struct {
 	SslPort     int    `toml:"ssl-port"`
 	SslCertPath string `toml:"ssl-cert"`
 	Port        int
+	ReadTimeout duration `toml:"read-timeout"`
 }
 
 type GraphiteConfig struct {
@@ -188,6 +192,7 @@ type Configuration struct {
 	ApiHttpSslPort               int
 	ApiHttpCertPath              string
 	ApiHttpPort                  int
+	ApiReadTimeout               time.Duration
 	GraphiteEnabled              bool
 	GraphitePort                 int
 	GraphiteDatabase             string
@@ -266,12 +271,18 @@ func parseTomlConfiguration(filename string) (*Configuration, error) {
 		tomlConfiguration.Raft.Timeout = duration{time.Second}
 	}
 
+	apiReadTimeout := tomlConfiguration.HttpApi.ReadTimeout.Duration
+	if apiReadTimeout == 0 {
+		apiReadTimeout = 5 * time.Second
+	}
+
 	config := &Configuration{
 		AdminHttpPort:                tomlConfiguration.Admin.Port,
 		AdminAssetsDir:               tomlConfiguration.Admin.Assets,
 		ApiHttpPort:                  tomlConfiguration.HttpApi.Port,
 		ApiHttpCertPath:              tomlConfiguration.HttpApi.SslCertPath,
 		ApiHttpSslPort:               tomlConfiguration.HttpApi.SslPort,
+		ApiReadTimeout:               apiReadTimeout,
 		GraphiteEnabled:              tomlConfiguration.InputPlugins.Graphite.Enabled,
 		GraphitePort:                 tomlConfiguration.InputPlugins.Graphite.Port,
 		GraphiteDatabase:             tomlConfiguration.InputPlugins.Graphite.Database,

src/server/server.go

@@ -55,7 +55,7 @@ func NewServer(config *configuration.Configuration) (*Server, error) {
 	protobufServer := coordinator.NewProtobufServer(config.ProtobufPortString(), requestHandler)
 
 	raftServer.AssignCoordinator(coord)
-	httpApi := http.NewHttpServer(config.ApiHttpPortString(), config.AdminAssetsDir, coord, coord, clusterConfig, raftServer)
+	httpApi := http.NewHttpServer(config.ApiHttpPortString(), config.ApiReadTimeout, config.AdminAssetsDir, coord, coord, clusterConfig, raftServer)
 	httpApi.EnableSsl(config.ApiHttpSslPortString(), config.ApiHttpCertPath)
 	graphiteApi := graphite.NewServer(config, coord, clusterConfig)
 	adminServer := admin.NewHttpServer(config.AdminAssetsDir, config.AdminHttpPortString())