diff --git a/storage/aws/antispam/aws.go b/storage/aws/antispam/aws.go index 2c6332fde..57b6d4818 100644 --- a/storage/aws/antispam/aws.go +++ b/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) } diff --git a/storage/aws/antispam/aws_test.go b/storage/aws/antispam/aws_test.go index 3c88d0088..c0cd67014 100644 --- a/storage/aws/antispam/aws_test.go +++ b/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. diff --git a/storage/aws/aws.go b/storage/aws/aws.go index f4c0aa26c..960510940 100644 --- a/storage/aws/aws.go +++ b/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) } diff --git a/storage/aws/aws_test.go b/storage/aws/aws_test.go index 1eee0ddc1..4069a8b6c 100644 --- a/storage/aws/aws_test.go +++ b/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) {