From 9f14a9389cab48b4c5aaad0aee2d5c8e09af897e Mon Sep 17 00:00:00 2001
From: Adhityaa Chandrasekar <adtac@adtac.in>
Date: Thu, 16 Aug 2018 23:12:21 +0530
Subject: [PATCH] api: add MAX_IDLE_PG_CONNECTIONS config variable

This allows lib/pq to maintain that many idle connections to the
PostgreSQL server. The larger the number the better, but obviously, your
machine needs to be capable to maintain that many connections.

Closes https://gitlab.com/commento/commento-ce/issues/76
---
 api/config.go           | 17 ++++++++++++++++-
 api/config_test.go      | 29 +++++++++++++++++++++++++++++
 api/database_connect.go | 14 ++++++++------
 3 files changed, 53 insertions(+), 7 deletions(-)

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
 }