storage/aws: keep idle MySQL connections by default

storage/aws/antispam/aws.go

@@ -72,8 +72,13 @@ type AntispamOpts struct {
 	PushbackThreshold uint
 
 	PushbackMaxOutstanding uint64
-	MaxOpenConns           int
-	MaxIdleConns           int
+	// MaxOpenConns is the maximum number of open connections to the antispam MySQL database.
+	// If zero or negative, the number of open connections is not limited.
+	MaxOpenConns int
+	// MaxIdleConns is the maximum number of idle connections kept in the antispam MySQL connection pool.
+	// If zero, the database/sql default is used (currently 2).
+	// If negative, no idle connections are kept, so every query opens a new connection.
+	MaxIdleConns int
 }
 
 type AntispamStorage struct {
@@ -115,7 +120,8 @@ func NewAntispam(ctx context.Context, dsn string, opts AntispamOpts) (*AntispamS
 	if opts.MaxOpenConns > 0 {
 		dbPool.SetMaxOpenConns(opts.MaxOpenConns)
 	}
-	if opts.MaxIdleConns >= 0 {
+	// Zero keeps the database/sql default; a negative value keeps no idle connections.
+	if opts.MaxIdleConns != 0 {
 		dbPool.SetMaxIdleConns(opts.MaxIdleConns)
 	}
 

storage/aws/antispam/aws_test.go

@@ -192,6 +192,44 @@ func TestAntispamIndexError(t *testing.T) {
 	}
 }
 
+// TestAntispamIdleConnections checks that antispam storage created without a
+// MaxIdleConns setting keeps connections for reuse, rather than closing each one
+// after use and opening a new one for the next query, and that a negative
+// MaxIdleConns keeps none.
+func TestAntispamIdleConnections(t *testing.T) {
+	ctx := context.Background()
+	if canSkipMySQLTest(t, ctx) {
+		slog.WarnContext(ctx, "MySQL not available, skipping", slog.String("name", t.Name()))
+		t.Skip("MySQL not available, skipping test")
+	}
+
+	for _, test := range []struct {
+		desc         string
+		maxIdleConns int
+		wantClosed   int64
+	}{
+		{desc: "unset keeps idle connections", maxIdleConns: 0, wantClosed: 0},
+		{desc: "negative keeps none", maxIdleConns: -1, wantClosed: 10},
+	} {
+		t.Run(test.desc, func(t *testing.T) {
+			as, err := NewAntispam(ctx, *mySQLURI, AntispamOpts{MaxIdleConns: test.maxIdleConns})
+			if err != nil {
+				t.Fatalf("NewAntispam: %v", err)
+			}
+			before := as.dbPool.Stats().MaxIdleClosed
+			for range 10 {
+				var one int
+				if err := as.dbPool.QueryRowContext(ctx, "SELECT 1").Scan(&one); err != nil {
+					t.Fatalf("query: %v", err)
+				}
+			}
+			if closed := as.dbPool.Stats().MaxIdleClosed - before; closed != test.wantClosed {
+				t.Errorf("%d of 10 sequential queries closed their connection, want %d", closed, test.wantClosed)
+			}
+		})
+	}
+}
+
 // canSkipMySQLTest checks if the test MySQL db is available and, if not, if the test can be skipped.
 //
 // Use this method before every MySQL test, and if it returns true, skip the test.

storage/aws/aws.go

@@ -158,9 +158,12 @@ type Config struct {
 	BucketPrefix string
 	// DSN is the DSN of the MySQL instance to use.
 	DSN string
-	// Maximum connections to the MySQL database.
+	// MaxOpenConns is the maximum number of open connections to the MySQL database.
+	// If zero or negative, the number of open connections is not limited.
 	MaxOpenConns int
-	// Maximum idle database connections in the connection pool.
+	// MaxIdleConns is the maximum number of idle connections kept in the MySQL connection pool.
+	// If zero, the database/sql default is used (currently 2).
+	// If negative, no idle connections are kept, so every query opens a new connection.
 	MaxIdleConns int
 
 	// HTTPClient will be used for other HTTP requests. If unset, Tessera will use the net/http DefaultClient.
@@ -907,7 +910,8 @@ func newMySQLSequencer(ctx context.Context, dsn string, maxOutstanding uint64, m
 	if maxOpenConns > 0 {
 		dbPool.SetMaxOpenConns(maxOpenConns)
 	}
-	if maxIdleConns >= 0 {
+	// Zero keeps the database/sql default; a negative value keeps no idle connections.
+	if maxIdleConns != 0 {
 		dbPool.SetMaxIdleConns(maxIdleConns)
 	}
 

storage/aws/aws_test.go

@@ -150,6 +150,45 @@ func TestMySQLSequencerAssignEntries(t *testing.T) {
 	}
 }
 
+// TestMySQLSequencerIdleConnections checks that a sequencer created without a
+// maxIdleConns setting keeps connections for reuse, rather than closing each one
+// after use and opening a new one for the next query, and that a negative
+// maxIdleConns keeps none.
+func TestMySQLSequencerIdleConnections(t *testing.T) {
+	ctx := context.Background()
+	if canSkipMySQLTest(t, ctx) {
+		slog.WarnContext(ctx, "MySQL not available, skipping", slog.String("name", t.Name()))
+		t.Skip("MySQL not available, skipping test")
+	}
+	mustDropTables(t, ctx)
+
+	for _, test := range []struct {
+		desc         string
+		maxIdleConns int
+		wantClosed   int64
+	}{
+		{desc: "unset keeps idle connections", maxIdleConns: 0, wantClosed: 0},
+		{desc: "negative keeps none", maxIdleConns: -1, wantClosed: 10},
+	} {
+		t.Run(test.desc, func(t *testing.T) {
+			seq, err := newMySQLSequencer(ctx, *mySQLURI, 1000, 0, test.maxIdleConns)
+			if err != nil {
+				t.Fatalf("newMySQLSequencer: %v", err)
+			}
+			before := seq.dbPool.Stats().MaxIdleClosed
+			for range 10 {
+				var one int
+				if err := seq.dbPool.QueryRowContext(ctx, "SELECT 1").Scan(&one); err != nil {
+					t.Fatalf("query: %v", err)
+				}
+			}
+			if closed := seq.dbPool.Stats().MaxIdleClosed - before; closed != test.wantClosed {
+				t.Errorf("%d of 10 sequential queries closed their connection, want %d", closed, test.wantClosed)
+			}
+		})
+	}
+}
+
 func TestMySQLSequencerPushback(t *testing.T) {
 	ctx := context.Background()
 	if canSkipMySQLTest(t, ctx) {