fix: Move SQL connection limits to initialization (#4981)

kylecarbs · web-flow · commit 089659ffb167 · 2022-11-09T16:25:25.000Z
The connection limit wasn't being applied to pubsub, which would
overload the server if a ton of logs were being published at once.

This should fix it, and improve scale a lot!
diff --git a/cli/server.go b/cli/server.go
@@ -447,6 +447,24 @@ func Server(vip *viper.Viper, newAPI func(context.Context, *coderd.Options) (*co
 				if err != nil {
 					return xerrors.Errorf("migrate up: %w", err)
 				}
+				// The default is 0 but the request will fail with a 500 if the DB
+				// cannot accept new connections, so we try to limit that here.
+				// Requests will wait for a new connection instead of a hard error
+				// if a limit is set.
+				sqlDB.SetMaxOpenConns(10)
+				// Allow a max of 3 idle connections at a time. Lower values end up
+				// creating a lot of connection churn. Since each connection uses about
+				// 10MB of memory, we're allocating 30MB to Postgres connections per
+				// replica, but is better than causing Postgres to spawn a thread 15-20
+				// times/sec. PGBouncer's transaction pooling is not the greatest so
+				// it's not optimal for us to deploy.
+				//
+				// This was set to 10 before we started doing HA deployments, but 3 was
+				// later determined to be a better middle ground as to not use up all
+				// of PGs default connection limit while simultaneously avoiding a lot
+				// of connection churn.
+				sqlDB.SetMaxIdleConns(3)
+
 				options.Database = database.New(sqlDB)
 				options.Pubsub, err = database.NewPubsub(ctx, sqlDB, cfg.PostgresURL.Value)
 				if err != nil {
diff --git a/coderd/database/db.go b/coderd/database/db.go
@@ -42,25 +42,6 @@ type DBTX interface {
 // New creates a new database store using a SQL database connection.
 func New(sdb *sql.DB) Store {
 	dbx := sqlx.NewDb(sdb, "postgres")
-
-	// The default is 0 but the request will fail with a 500 if the DB
-	// cannot accept new connections, so we try to limit that here.
-	// Requests will wait for a new connection instead of a hard error
-	// if a limit is set.
-	dbx.SetMaxOpenConns(40)
-	// Allow a max of 3 idle connections at a time. Lower values end up
-	// creating a lot of connection churn. Since each connection uses about
-	// 10MB of memory, we're allocating 30MB to Postgres connections per
-	// replica, but is better than causing Postgres to spawn a thread 15-20
-	// times/sec. PGBouncer's transaction pooling is not the greatest so
-	// it's not optimal for us to deploy.
-	//
-	// This was set to 10 before we started doing HA deployments, but 3 was
-	// later determined to be a better middle ground as to not use up all
-	// of PGs default connection limit while simultaneously avoiding a lot
-	// of connection churn.
-	dbx.SetMaxIdleConns(3)
-
 	return &sqlQuerier{
 		db:  dbx,
 		sdb: dbx,
