diff --git a/api/config.go b/api/config.go index 89ab5e4..9ace4ab 100644 --- a/api/config.go +++ b/api/config.go @@ -3,6 +3,7 @@ package main import ( "os" "path/filepath" + "strconv" "strings" ) @@ -18,6 +19,12 @@ func configParse() error { "POSTGRES": "postgres://postgres:postgres@localhost/commento?sslmode=disable", + // PostgreSQL recommends max_connections in the order of hundreds. The default + // is 100, so let's use half that and leave the other half for other services. + // Ideally, you'd be setting this to a much higher number (for example, at the + // time of writing, commento.io uses 600). See https://wiki.postgresql.org/wiki/Number_Of_Database_Connections + "MAX_IDLE_PG_CONNECTIONS": "50", + "BIND_ADDRESS": "127.0.0.1", "PORT": "8080", "ORIGIN": "", @@ -55,7 +62,7 @@ func configParse() error { } // Mandatory config parameters - for _, env := range []string{"POSTGRES", "PORT", "ORIGIN", "FORBID_NEW_OWNERS"} { + for _, env := range []string{"POSTGRES", "PORT", "ORIGIN", "FORBID_NEW_OWNERS", "MAX_IDLE_PG_CONNECTIONS"} { if os.Getenv(env) == "" { logger.Errorf("missing COMMENTO_%s environment variable", env) return errorMissingConfig @@ -95,5 +102,13 @@ func configParse() error { os.Setenv("STATIC", static) + if num, err := strconv.Atoi(os.Getenv("MAX_IDLE_PG_CONNECTIONS")); err != nil { + logger.Errorf("invalid COMMENTO_MAX_IDLE_PG_CONNECTIONS: %v", err) + return errorInvalidConfigValue + } else if num <= 0 { + logger.Errorf("COMMENTO_MAX_IDLE_PG_CONNECTIONS should be a positive integer") + return errorInvalidConfigValue + } + return nil } diff --git a/api/config_test.go b/api/config_test.go index 605f465..bb11a73 100644 --- a/api/config_test.go +++ b/api/config_test.go @@ -125,3 +125,32 @@ func TestConfigOriginTrailingSlash(t *testing.T) { return } } + +func TestConfigMaxConnections(t *testing.T) { + os.Setenv("COMMENTO_ORIGIN", "https://commento.io") + os.Setenv("COMMENTO_STATIC", "") + + os.Setenv("COMMENTO_MAX_IDLE_PG_CONNECTIONS", "100") + if err := configParse(); err != nil { + t.Errorf("unexpected error when MAX_IDLE_PG_CONNECTIONS=100: %v", err) + return + } + + os.Setenv("COMMENTO_MAX_IDLE_PG_CONNECTIONS", "text") + if err := configParse(); err == nil { + t.Errorf("expected error with MAX_IDLE_PG_CONNECTIONS=text not found") + return + } + + os.Setenv("COMMENTO_MAX_IDLE_PG_CONNECTIONS", "0") + if err := configParse(); err == nil { + t.Errorf("expected error with MAX_IDLE_PG_CONNECTIONS=0 not found") + return + } + + os.Setenv("COMMENTO_MAX_IDLE_PG_CONNECTIONS", "-1") + if err := configParse(); err == nil { + t.Errorf("expected error with MAX_IDLE_PG_CONNECTIONS=-1 not found") + return + } +} diff --git a/api/database_connect.go b/api/database_connect.go index 3c2eb7d..69d8a4d 100644 --- a/api/database_connect.go +++ b/api/database_connect.go @@ -4,6 +4,7 @@ import ( "database/sql" _ "github.com/lib/pq" "os" + "strconv" "time" ) @@ -41,12 +42,13 @@ func dbConnect(retriesLeft int) error { return err } - // At most 1000 database connections will be left open in the idle state. This - // was found to be important when benchmarking with `wrk`: if this was unset, - // too many open idle connections were present, resulting in dropped requests - // due to the limit on the number of file handles. On benchmarking, around - // 100 was found to be pretty optimal. - db.SetMaxIdleConns(100) + maxIdleConnections, err := strconv.Atoi(os.Getenv("MAX_IDLE_PG_CONNECTIONS")) + if err != nil { + logger.Warningf("cannot parse COMMENTO_MAX_IDLE_PG_CONNECTIONS: %v", err) + maxIdleConnections = 50 + } + + db.SetMaxIdleConns(maxIdleConnections) return nil }