Merge pull request #31 from hellofresh/hotfix/my-pg-connections-leak

PT-4880 Fixed MySQL and PG connections leak

.travis.yml

@@ -5,6 +5,7 @@ services:
 
 go:
   - "1.11"
+  - "1.12"
   - "stable"
 
 before_install:

checks/http/check_test.go

@@ -3,20 +3,26 @@ package http
 import (
 	"os"
 	"testing"
+
+	"github.com/stretchr/testify/require"
 )
 
 const httpURLEnv = "HEALTH_GO_HTTP_URL"
 
-func TestNew(t *testing.T) {
-	if os.Getenv(httpURLEnv) == "" {
-		t.SkipNow()
+func getURL(t *testing.T) string {
+	if httpURL, ok := os.LookupEnv(httpURLEnv); ok {
+		return httpURL
 	}
 
+	t.Fatalf("required env variable missing: %s", httpURLEnv)
+	return ""
+}
+
+func TestNew(t *testing.T) {
 	check := New(Config{
-		URL: os.Getenv(httpURLEnv),
+		URL: getURL(t),
 	})
 
-	if err := check(); err != nil {
-		t.Fatalf("HTTP check failed: %s", err.Error())
-	}
+	err := check()
+	require.NoError(t, err)
 }

checks/mongo/check_test.go

@@ -3,20 +3,26 @@ package mongo
 import (
 	"os"
 	"testing"
+
+	"github.com/stretchr/testify/require"
 )
 
 const mgDSNEnv = "HEALTH_GO_MG_DSN"
 
-func TestNew(t *testing.T) {
-	if os.Getenv(mgDSNEnv) == "" {
-		t.SkipNow()
+func getDSN(t *testing.T) string {
+	if mongoDSN, ok := os.LookupEnv(mgDSNEnv); ok {
+		return mongoDSN
 	}
 
+	t.Fatalf("required env variable missing: %s", mgDSNEnv)
+	return ""
+}
+
+func TestNew(t *testing.T) {
 	check := New(Config{
-		DSN: os.Getenv(mgDSNEnv),
+		DSN: getDSN(t),
 	})
 
-	if err := check(); err != nil {
-		t.Fatalf("MongoDB check failed: %s", err.Error())
-	}
+	err := check()
+	require.NoError(t, err)
 }

checks/mysql/check.go

@@ -3,7 +3,6 @@ package mysql
 import (
 	"database/sql"
 
-	// MySQL database driver
 	_ "github.com/go-sql-driver/mysql"
 )
 
@@ -45,11 +44,16 @@ func New(config Config) func() error {
 			return err
 		}
 
-		_, err = db.Query(`SELECT VERSION()`)
+		rows, err := db.Query(`SELECT VERSION()`)
 		if err != nil {
 			config.LogFunc(err, "MySQL health check failed during select")
 			return err
 		}
+		defer func() {
+			if err = rows.Close(); err != nil {
+				config.LogFunc(err, "MySQL health check failed during rows closing")
+			}
+		}()
 
 		return nil
 	}

checks/mysql/check_test.go

@@ -1,22 +1,68 @@
 package mysql
 
 import (
+	"database/sql"
 	"os"
 	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 const mysqlDSNEnv = "HEALTH_GO_MS_DSN"
 
-func TestNew(t *testing.T) {
-	if os.Getenv(mysqlDSNEnv) == "" {
-		t.SkipNow()
+func getDSN(t *testing.T) string {
+	if mysqlDSN, ok := os.LookupEnv(mysqlDSNEnv); ok {
+		return mysqlDSN
 	}
 
+	t.Fatalf("required env variable missing: %s", mysqlDSNEnv)
+	return ""
+}
+
+func TestNew(t *testing.T) {
+	check := New(Config{
+		DSN: getDSN(t),
+	})
+
+	err := check()
+	require.NoError(t, err)
+}
+
+func TestEnsureConnectionIsClosed(t *testing.T) {
+	mysqlDSN := getDSN(t)
+
+	db, err := sql.Open("mysql", mysqlDSN)
+	require.NoError(t, err)
+
+	defer func() {
+		err := db.Close()
+		assert.NoError(t, err)
+	}()
+
+	var (
+		varName            string
+		initialConnections int
+	)
+	row := db.QueryRow(`SHOW STATUS WHERE variable_name = 'Threads_connected'`)
+	err = row.Scan(&varName, &initialConnections)
+	require.NoError(t, err)
+
 	check := New(Config{
-		DSN: os.Getenv(mysqlDSNEnv),
+		DSN: mysqlDSN,
 	})
 
-	if err := check(); err != nil {
-		t.Fatalf("MySQL check failed: %s", err.Error())
+	for i := 0; i < 10; i++ {
+		err := check()
+		assert.NoError(t, err)
+		time.Sleep(100 * time.Millisecond)
 	}
+
+	var currentConnections int
+	row = db.QueryRow(`SHOW STATUS WHERE variable_name = 'Threads_connected'`)
+	err = row.Scan(&varName, &currentConnections)
+	require.NoError(t, err)
+
+	assert.Equal(t, initialConnections, currentConnections)
 }

checks/postgres/check.go

@@ -2,6 +2,7 @@ package postgres
 
 import (
 	"database/sql"
+
 	_ "github.com/lib/pq"
 )
 
@@ -41,11 +42,16 @@ func New(config Config) func() error {
 			return err
 		}
 
-		_, err = db.Query(`SELECT VERSION()`)
+		rows, err := db.Query(`SELECT VERSION()`)
 		if err != nil {
 			config.LogFunc(err, "PostgreSQL health check failed during select")
 			return err
 		}
+		defer func() {
+			if err = rows.Close(); err != nil {
+				config.LogFunc(err, "PostgreSQL health check failed during rows closing")
+			}
+		}()
 
 		return nil
 	}

checks/postgres/check_test.go

@@ -1,22 +1,66 @@
 package postgres
 
 import (
+	"database/sql"
 	"os"
 	"testing"
+	"time"
+
+	_ "github.com/lib/pq"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 const pgDSNEnv = "HEALTH_GO_PG_DSN"
 
-func TestNew(t *testing.T) {
-	if os.Getenv(pgDSNEnv) == "" {
-		t.SkipNow()
+func getDSN(t *testing.T) string {
+	if pgDSN, ok := os.LookupEnv(pgDSNEnv); ok {
+		return pgDSN
 	}
 
+	t.Fatalf("required env variable missing: %s", pgDSNEnv)
+	return ""
+}
+
+func TestNew(t *testing.T) {
+	check := New(Config{
+		DSN: getDSN(t),
+	})
+
+	err := check()
+	require.NoError(t, err)
+}
+
+func TestEnsureConnectionIsClosed(t *testing.T) {
+	pgDSN := getDSN(t)
+
+	db, err := sql.Open("postgres", pgDSN)
+	require.NoError(t, err)
+
+	defer func() {
+		err := db.Close()
+		assert.NoError(t, err)
+	}()
+
+	var initialConnections int
+	row := db.QueryRow(`SELECT sum(numbackends) FROM pg_stat_database`)
+	err = row.Scan(&initialConnections)
+	require.NoError(t, err)
+
 	check := New(Config{
-		DSN: os.Getenv(pgDSNEnv),
+		DSN: pgDSN,
 	})
 
-	if err := check(); err != nil {
-		t.Fatalf("PostgreSQL check failed: %s", err.Error())
+	for i := 0; i < 10; i++ {
+		err := check()
+		assert.NoError(t, err)
+		time.Sleep(100 * time.Millisecond)
 	}
+
+	var currentConnections int
+	row = db.QueryRow(`SELECT sum(numbackends) FROM pg_stat_database`)
+	err = row.Scan(&currentConnections)
+	require.NoError(t, err)
+
+	assert.Equal(t, initialConnections, currentConnections)
 }

checks/rabbitmq/aliveness_check_test.go

@@ -5,20 +5,25 @@ import (
 	"testing"
 
 	"github.com/hellofresh/health-go/checks/http"
+	"github.com/stretchr/testify/require"
 )
 
 const httpURLEnv = "HEALTH_GO_MQ_URL"
 
-func TestAliveness(t *testing.T) {
-	if os.Getenv(httpURLEnv) == "" {
-		t.SkipNow()
+func getURL(t *testing.T) string {
+	if httpURL, ok := os.LookupEnv(httpURLEnv); ok {
+		return httpURL
 	}
 
+	t.Fatalf("required env variable missing: %s", httpURLEnv)
+	return ""
+}
+
+func TestAliveness(t *testing.T) {
 	check := http.New(http.Config{
-		URL: os.Getenv(httpURLEnv),
+		URL: getURL(t),
 	})
 
-	if err := check(); err != nil {
-		t.Fatalf("HTTP check failed: %s", err.Error())
-	}
+	err := check()
+	require.NoError(t, err)
 }

checks/rabbitmq/check_test.go

@@ -3,36 +3,38 @@ package rabbitmq
 import (
 	"os"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 const mqDSNEnv = "HEALTH_GO_MQ_DSN"
 
-func TestNew(t *testing.T) {
-	if os.Getenv(mqDSNEnv) == "" {
-		t.SkipNow()
+func getDSN(t *testing.T) string {
+	if mqDSN, ok := os.LookupEnv(mqDSNEnv); ok {
+		return mqDSN
 	}
 
+	t.Fatalf("required env variable missing: %s", mqDSNEnv)
+	return ""
+}
+
+func TestNew(t *testing.T) {
 	check := New(Config{
-		DSN: os.Getenv(mqDSNEnv),
+		DSN: getDSN(t),
 	})
 
-	if err := check(); err != nil {
-		t.Fatalf("RabbitMQ check failed: %s", err.Error())
-	}
+	err := check()
+	require.NoError(t, err)
 }
 
 func TestConfig(t *testing.T) {
 	conf := &Config{
-		DSN: os.Getenv(mqDSNEnv),
+		DSN: getDSN(t),
 	}
 
 	conf.defaults()
 
-	if conf.Exchange != defaultExchange {
-		t.Fatal("Invalid default conf exchange value")
-	}
-
-	if conf.ConsumeTimeout != defaultConsumeTimeout {
-		t.Fatal("Invalid default conf exchange value")
-	}
+	assert.Equal(t, defaultExchange, conf.Exchange)
+	assert.Equal(t, defaultConsumeTimeout, conf.ConsumeTimeout)
 }

checks/redis/check_test.go

@@ -3,20 +3,26 @@ package redis
 import (
 	"os"
 	"testing"
+
+	"github.com/stretchr/testify/require"
 )
 
 const rdDSNEnv = "HEALTH_GO_RD_DSN"
 
-func TestNew(t *testing.T) {
-	if os.Getenv(rdDSNEnv) == "" {
-		t.SkipNow()
+func getDSN(t *testing.T) string {
+	if redisDSN, ok := os.LookupEnv(rdDSNEnv); ok {
+		return redisDSN
 	}
 
+	t.Fatalf("required env variable missing: %s", rdDSNEnv)
+	return ""
+}
+
+func TestNew(t *testing.T) {
 	check := New(Config{
-		DSN: os.Getenv(rdDSNEnv),
+		DSN: getDSN(t),
 	})
 
-	if err := check(); err != nil {
-		t.Fatalf("Redis check failed: %s", err.Error())
-	}
+	err := check()
+	require.NoError(t, err)
 }