diff --git a/cmd/admin-revoker/main.go b/cmd/admin-revoker/main.go index fc32e988a..2c93353ce 100644 --- a/cmd/admin-revoker/main.go +++ b/cmd/admin-revoker/main.go @@ -75,7 +75,13 @@ func setupContext(c config) (core.RegistrationAuthority, blog.Logger, *db.Wrappe dbURL, err := c.Revoker.DBConfig.URL() cmd.FailOnError(err, "Couldn't load DB URL") - dbMap, err := sa.NewDbMap(dbURL, c.Revoker.DBConfig.MaxDBConns) + dbSettings := sa.DbSettings{ + MaxOpenConns: c.Revoker.DBConfig.GetMaxOpenConns(), + MaxIdleConns: c.Revoker.DBConfig.MaxIdleConns, + ConnMaxLifetime: c.Revoker.DBConfig.ConnMaxLifetime.Duration, + ConnMaxIdleTime: c.Revoker.DBConfig.ConnMaxIdleTime.Duration, + } + dbMap, err := sa.NewDbMap(dbURL, dbSettings) cmd.FailOnError(err, "Couldn't setup database connection") saConn, err := bgrpc.ClientSetup(c.Revoker.SAService, tlsConfig, clientMetrics, clk) diff --git a/cmd/admin-revoker/main_test.go b/cmd/admin-revoker/main_test.go index 9a27e654d..57590a338 100644 --- a/cmd/admin-revoker/main_test.go +++ b/cmd/admin-revoker/main_test.go @@ -41,7 +41,7 @@ func TestRevokeBatch(t *testing.T) { fc := clock.NewFake() // Set to some non-zero time. fc.Set(time.Date(2015, 3, 4, 5, 0, 0, 0, time.UTC)) - dbMap, err := sa.NewDbMap(vars.DBConnSA, 0) + dbMap, err := sa.NewDbMap(vars.DBConnSA, sa.DbSettings{}) if err != nil { t.Fatalf("Failed to create dbMap: %s", err) } diff --git a/cmd/bad-key-revoker/main.go b/cmd/bad-key-revoker/main.go index 563ce678c..582357356 100644 --- a/cmd/bad-key-revoker/main.go +++ b/cmd/bad-key-revoker/main.go @@ -391,10 +391,17 @@ func main() { dbURL, err := config.BadKeyRevoker.DBConfig.URL() cmd.FailOnError(err, "Couldn't load DB URL") - dbMap, err := sa.NewDbMap(dbURL, config.BadKeyRevoker.DBConfig.MaxDBConns) + + dbSettings := sa.DbSettings{ + MaxOpenConns: config.BadKeyRevoker.DBConfig.GetMaxOpenConns(), + MaxIdleConns: config.BadKeyRevoker.DBConfig.MaxIdleConns, + ConnMaxLifetime: config.BadKeyRevoker.DBConfig.ConnMaxLifetime.Duration, + ConnMaxIdleTime: config.BadKeyRevoker.DBConfig.ConnMaxIdleTime.Duration, + } + dbMap, err := sa.NewDbMap(dbURL, dbSettings) cmd.FailOnError(err, "Could not connect to database") sa.SetSQLDebug(dbMap, logger) - sa.InitDBMetrics(dbMap, scope) + sa.InitDBMetrics(dbMap, scope, dbSettings) tlsConfig, err := config.BadKeyRevoker.TLS.Load() cmd.FailOnError(err, "TLS config") diff --git a/cmd/bad-key-revoker/main_test.go b/cmd/bad-key-revoker/main_test.go index a52c9627d..05623e6b1 100644 --- a/cmd/bad-key-revoker/main_test.go +++ b/cmd/bad-key-revoker/main_test.go @@ -46,7 +46,7 @@ func insertBlockedRow(t *testing.T, dbMap *db.WrappedMap, hash []byte, by int64, } func TestSelectUncheckedRows(t *testing.T) { - dbMap, err := sa.NewDbMap(vars.DBConnSAFullPerms, 0) + dbMap, err := sa.NewDbMap(vars.DBConnSAFullPerms, sa.DbSettings{}) test.AssertNotError(t, err, "failed setting up db client") defer test.ResetSATestDatabase(t)() @@ -159,7 +159,7 @@ func insertCert(t *testing.T, dbMap *db.WrappedMap, keyHash []byte, serial strin // does not have a corresponding entry in the certificateStatus and // precertificates table. func TestFindUnrevokedNoRows(t *testing.T) { - dbMap, err := sa.NewDbMap(vars.DBConnSAFullPerms, 0) + dbMap, err := sa.NewDbMap(vars.DBConnSAFullPerms, sa.DbSettings{}) test.AssertNotError(t, err, "failed setting up db client") defer test.ResetSATestDatabase(t)() @@ -178,7 +178,7 @@ func TestFindUnrevokedNoRows(t *testing.T) { } func TestFindUnrevoked(t *testing.T) { - dbMap, err := sa.NewDbMap(vars.DBConnSAFullPerms, 0) + dbMap, err := sa.NewDbMap(vars.DBConnSAFullPerms, sa.DbSettings{}) test.AssertNotError(t, err, "failed setting up db client") defer test.ResetSATestDatabase(t)() @@ -208,7 +208,7 @@ func TestFindUnrevoked(t *testing.T) { } func TestResolveContacts(t *testing.T) { - dbMap, err := sa.NewDbMap(vars.DBConnSAFullPerms, 0) + dbMap, err := sa.NewDbMap(vars.DBConnSAFullPerms, sa.DbSettings{}) test.AssertNotError(t, err, "failed setting up db client") defer test.ResetSATestDatabase(t)() @@ -258,7 +258,7 @@ func (mr *mockRevoker) AdministrativelyRevokeCertificate(ctx context.Context, in } func TestRevokeCerts(t *testing.T) { - dbMap, err := sa.NewDbMap(vars.DBConnSAFullPerms, 0) + dbMap, err := sa.NewDbMap(vars.DBConnSAFullPerms, sa.DbSettings{}) test.AssertNotError(t, err, "failed setting up db client") defer test.ResetSATestDatabase(t)() @@ -279,7 +279,7 @@ func TestRevokeCerts(t *testing.T) { } func TestCertificateAbsent(t *testing.T) { - dbMap, err := sa.NewDbMap(vars.DBConnSAFullPerms, 0) + dbMap, err := sa.NewDbMap(vars.DBConnSAFullPerms, sa.DbSettings{}) test.AssertNotError(t, err, "failed setting up db client") defer test.ResetSATestDatabase(t)() @@ -313,7 +313,7 @@ func TestCertificateAbsent(t *testing.T) { } func TestInvoke(t *testing.T) { - dbMap, err := sa.NewDbMap(vars.DBConnSAFullPerms, 0) + dbMap, err := sa.NewDbMap(vars.DBConnSAFullPerms, sa.DbSettings{}) test.AssertNotError(t, err, "failed setting up db client") defer test.ResetSATestDatabase(t)() @@ -381,7 +381,7 @@ func TestInvokeRevokerHasNoExtantCerts(t *testing.T) { // extant certificates themselves their contact email is still // resolved and we avoid sending any emails to accounts that // share the same email. - dbMap, err := sa.NewDbMap(vars.DBConnSAFullPerms, 0) + dbMap, err := sa.NewDbMap(vars.DBConnSAFullPerms, sa.DbSettings{}) test.AssertNotError(t, err, "failed setting up db client") defer test.ResetSATestDatabase(t)() diff --git a/cmd/boulder-janitor/janitor/handlers_test.go b/cmd/boulder-janitor/janitor/handlers_test.go index 07f73c8b5..8389ec6c9 100644 --- a/cmd/boulder-janitor/janitor/handlers_test.go +++ b/cmd/boulder-janitor/janitor/handlers_test.go @@ -23,7 +23,7 @@ func TestDeleteOrder(t *testing.T) { log, fc := setup() // Create one dbMap for the SA with the SA user. - dbMap, err := sa.NewDbMap(vars.DBConnSA, 0) + dbMap, err := sa.NewDbMap(vars.DBConnSA, sa.DbSettings{}) test.AssertNotError(t, err, "error creating db map") // Create a SSA backed by the SA user dbMap ssa, err := sa.NewSQLStorageAuthority(dbMap, fc, log, metrics.NoopRegisterer, 1) @@ -87,7 +87,7 @@ func TestDeleteOrder(t *testing.T) { // Create a dbMap for the janitor user. We don't want to use the SA dbMap // because it doesn't have DELETE grants. // Create one dbMap for the SA with the SA user. - janitorDbMap, err := sa.NewDbMap("janitor@tcp(boulder-mysql:3306)/boulder_sa_test", 0) + janitorDbMap, err := sa.NewDbMap("janitor@tcp(boulder-mysql:3306)/boulder_sa_test", sa.DbSettings{}) test.AssertNotError(t, err, "error creating db map") // Create an Orders job diff --git a/cmd/boulder-janitor/janitor/janitor.go b/cmd/boulder-janitor/janitor/janitor.go index 156284225..37dc980aa 100644 --- a/cmd/boulder-janitor/janitor/janitor.go +++ b/cmd/boulder-janitor/janitor/janitor.go @@ -67,7 +67,13 @@ func New(clk clock.Clock, config JanitorConfig) (*Janitor, error) { if err != nil { return nil, err } - dbMap, err := sa.NewDbMap(dbURL, config.DBConfig.MaxDBConns) + dbSettings := sa.DbSettings{ + MaxOpenConns: config.DBConfig.GetMaxOpenConns(), + MaxIdleConns: config.DBConfig.MaxIdleConns, + ConnMaxLifetime: config.DBConfig.ConnMaxLifetime.Duration, + ConnMaxIdleTime: config.DBConfig.ConnMaxIdleTime.Duration, + } + dbMap, err := sa.NewDbMap(dbURL, dbSettings) if err != nil { return nil, err } diff --git a/cmd/boulder-sa/main.go b/cmd/boulder-sa/main.go index e66a5e83b..1f7f029a6 100644 --- a/cmd/boulder-sa/main.go +++ b/cmd/boulder-sa/main.go @@ -57,15 +57,21 @@ func main() { logger.Info(cmd.VersionString()) saConf := c.SA + saDbSettings := sa.DbSettings{ + MaxOpenConns: saConf.DBConfig.GetMaxOpenConns(), + MaxIdleConns: saConf.DBConfig.MaxIdleConns, + ConnMaxLifetime: saConf.DBConfig.ConnMaxLifetime.Duration, + ConnMaxIdleTime: saConf.DBConfig.ConnMaxIdleTime.Duration, + } dbURL, err := saConf.DBConfig.URL() cmd.FailOnError(err, "Couldn't load DB URL") - dbMap, err := sa.NewDbMap(dbURL, saConf.DBConfig.MaxDBConns) + dbMap, err := sa.NewDbMap(dbURL, saDbSettings) cmd.FailOnError(err, "Couldn't connect to SA database") // Collect and periodically report DB metrics using the DBMap and prometheus scope. - sa.InitDBMetrics(dbMap, scope) + sa.InitDBMetrics(dbMap, scope, saDbSettings) clk := cmd.Clock() diff --git a/cmd/cert-checker/main.go b/cmd/cert-checker/main.go index 236853427..641de352f 100644 --- a/cmd/cert-checker/main.go +++ b/cmd/cert-checker/main.go @@ -373,10 +373,16 @@ func main() { saDbURL, err := config.CertChecker.DBConfig.URL() cmd.FailOnError(err, "Couldn't load DB URL") - saDbMap, err := sa.NewDbMap(saDbURL, config.CertChecker.DBConfig.MaxDBConns) + dbSettings := sa.DbSettings{ + MaxOpenConns: config.CertChecker.DBConfig.GetMaxOpenConns(), + MaxIdleConns: config.CertChecker.DBConfig.MaxIdleConns, + ConnMaxLifetime: config.CertChecker.DBConfig.ConnMaxLifetime.Duration, + ConnMaxIdleTime: config.CertChecker.DBConfig.ConnMaxIdleTime.Duration, + } + saDbMap, err := sa.NewDbMap(saDbURL, dbSettings) cmd.FailOnError(err, "Could not connect to database") - sa.InitDBMetrics(saDbMap, prometheus.DefaultRegisterer) + sa.InitDBMetrics(saDbMap, prometheus.DefaultRegisterer, dbSettings) checkerLatency := prometheus.NewHistogram(prometheus.HistogramOpts{ Name: "cert_checker_latency", diff --git a/cmd/cert-checker/main_test.go b/cmd/cert-checker/main_test.go index a6927fc9b..cf6499a7a 100644 --- a/cmd/cert-checker/main_test.go +++ b/cmd/cert-checker/main_test.go @@ -44,7 +44,7 @@ func init() { } func BenchmarkCheckCert(b *testing.B) { - saDbMap, err := sa.NewDbMap(vars.DBConnSA, 0) + saDbMap, err := sa.NewDbMap(vars.DBConnSA, sa.DbSettings{}) if err != nil { fmt.Println("Couldn't connect to database") return @@ -80,7 +80,7 @@ func BenchmarkCheckCert(b *testing.B) { } func TestCheckWildcardCert(t *testing.T) { - saDbMap, err := sa.NewDbMap(vars.DBConnSA, 0) + saDbMap, err := sa.NewDbMap(vars.DBConnSA, sa.DbSettings{}) test.AssertNotError(t, err, "Couldn't connect to database") saCleanup := test.ResetSATestDatabase(t) defer func() { @@ -127,7 +127,7 @@ func TestCheckWildcardCert(t *testing.T) { } func TestCheckCert(t *testing.T) { - saDbMap, err := sa.NewDbMap(vars.DBConnSA, 0) + saDbMap, err := sa.NewDbMap(vars.DBConnSA, sa.DbSettings{}) test.AssertNotError(t, err, "Couldn't connect to database") saCleanup := test.ResetSATestDatabase(t) defer func() { @@ -252,7 +252,7 @@ func TestCheckCert(t *testing.T) { } func TestGetAndProcessCerts(t *testing.T) { - saDbMap, err := sa.NewDbMap(vars.DBConnSA, 0) + saDbMap, err := sa.NewDbMap(vars.DBConnSA, sa.DbSettings{}) test.AssertNotError(t, err, "Couldn't connect to database") fc := clock.NewFake() @@ -337,7 +337,7 @@ func (db mismatchedCountDB) Select(output interface{}, _ string, _ ...interface{ * 0: https://github.com/letsencrypt/boulder/issues/2004 */ func TestGetCertsEmptyResults(t *testing.T) { - saDbMap, err := sa.NewDbMap(vars.DBConnSA, 0) + saDbMap, err := sa.NewDbMap(vars.DBConnSA, sa.DbSettings{}) test.AssertNotError(t, err, "Couldn't connect to database") fc := clock.NewFake() checker := newChecker(saDbMap, fc, pa, expectedValidityPeriod) @@ -413,7 +413,7 @@ func TestIsForbiddenDomain(t *testing.T) { } func TestIgnoredLint(t *testing.T) { - saDbMap, err := sa.NewDbMap(vars.DBConnSA, 0) + saDbMap, err := sa.NewDbMap(vars.DBConnSA, sa.DbSettings{}) test.AssertNotError(t, err, "Couldn't connect to database") saCleanup := test.ResetSATestDatabase(t) defer func() { diff --git a/cmd/config.go b/cmd/config.go index 31b918dd2..7293bb64c 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -49,7 +49,39 @@ type DBConfig struct { DBConnect string // A file containing a connect URL for the DB. DBConnectFile string - MaxDBConns int + + // MaxDBConns sets the maximum number of open connections to the + // database. If MaxIdleConns is greater than 0 and MaxOpenConns is + // less than MaxIdleConns, then MaxIdleConns will be reduced to + // match the new MaxOpenConns limit. If n < 0, then there is no + // limit on the number of open connections. + // TODO(#5249): This field can be removed once MaxDBConns has been + // removed from test/config and all prod and staging configs. + MaxDBConns int + + // MaxOpenConns sets the maximum number of open connections to the + // database. If MaxIdleConns is greater than 0 and MaxOpenConns is + // less than MaxIdleConns, then MaxIdleConns will be reduced to + // match the new MaxOpenConns limit. If n < 0, then there is no + // limit on the number of open connections. + MaxOpenConns int + + // MaxIdleConns sets the maximum number of connections in the idle + // connection pool. If MaxOpenConns is greater than 0 but less than + // MaxIdleConns, then MaxIdleConns will be reduced to match the + // MaxOpenConns limit. If n < 0, no idle connections are retained. + MaxIdleConns int + + // ConnMaxLifetime sets the maximum amount of time a connection may + // be reused. Expired connections may be closed lazily before reuse. + // If d < 0, connections are not closed due to a connection's age. + ConnMaxLifetime ConfigDuration + + // ConnMaxIdleTime sets the maximum amount of time a connection may + // be idle. Expired connections may be closed lazily before reuse. + // If d < 0, connections are not closed due to a connection's idle + // time. + ConnMaxIdleTime ConfigDuration } // URL returns the DBConnect URL represented by this DBConfig object, either @@ -63,6 +95,17 @@ func (d *DBConfig) URL() (string, error) { return d.DBConnect, nil } +// GetMaxOpenConns defaults MaxOpenConns to the value of MaxDBConns if +// MaxDBConns was specified in the config json. +// TODO(#5249): This method can be removed once MaxDBConns has been +// removed from test/config and all prod and staging configs. +func (d *DBConfig) GetMaxOpenConns() int { + if d.MaxOpenConns == 0 { + d.MaxOpenConns = d.MaxDBConns + } + return d.MaxOpenConns +} + type SMTPConfig struct { PasswordConfig Server string diff --git a/cmd/config_test.go b/cmd/config_test.go index 0078ee5d6..d20e1a66b 100644 --- a/cmd/config_test.go +++ b/cmd/config_test.go @@ -32,6 +32,39 @@ func TestDBConfigURL(t *testing.T) { } } +func TestGetMaxOpenConns(t *testing.T) { + tests := []struct { + conf DBConfig + expected int + }{ + { + // Test with config that contains both fields with different values + conf: DBConfig{MaxDBConns: 1, MaxOpenConns: 100}, + expected: 100, + }, + { + // Test with config that contains only MaxDBConns + conf: DBConfig{MaxDBConns: 100}, + expected: 100, + }, + { + // Test with config that contains only MaxOpenConns + conf: DBConfig{MaxOpenConns: 1}, + expected: 1, + }, + { + // Test with config that contains neither field + conf: DBConfig{}, + expected: 0, + }, + } + + for _, tc := range tests { + maxOpenConns := tc.conf.GetMaxOpenConns() + test.AssertEquals(t, maxOpenConns, tc.expected) + } +} + func TestPasswordConfig(t *testing.T) { tests := []struct { pc PasswordConfig diff --git a/cmd/expiration-mailer/main.go b/cmd/expiration-mailer/main.go index f444c3288..5b5e29cb7 100644 --- a/cmd/expiration-mailer/main.go +++ b/cmd/expiration-mailer/main.go @@ -477,12 +477,18 @@ func main() { // Configure DB dbURL, err := c.Mailer.DBConfig.URL() cmd.FailOnError(err, "Couldn't load DB URL") - dbMap, err := sa.NewDbMap(dbURL, c.Mailer.DBConfig.MaxDBConns) + dbSettings := sa.DbSettings{ + MaxOpenConns: c.Mailer.DBConfig.GetMaxOpenConns(), + MaxIdleConns: c.Mailer.DBConfig.MaxIdleConns, + ConnMaxLifetime: c.Mailer.DBConfig.ConnMaxLifetime.Duration, + ConnMaxIdleTime: c.Mailer.DBConfig.ConnMaxIdleTime.Duration, + } + dbMap, err := sa.NewDbMap(dbURL, dbSettings) cmd.FailOnError(err, "Could not connect to database") sa.SetSQLDebug(dbMap, logger) // Collect and periodically report DB metrics using the DBMap and prometheus scope. - sa.InitDBMetrics(dbMap, scope) + sa.InitDBMetrics(dbMap, scope, dbSettings) tlsConfig, err := c.Mailer.TLS.Load() cmd.FailOnError(err, "TLS config") diff --git a/cmd/expiration-mailer/main_test.go b/cmd/expiration-mailer/main_test.go index 7729cde6d..3679c3a7f 100644 --- a/cmd/expiration-mailer/main_test.go +++ b/cmd/expiration-mailer/main_test.go @@ -374,7 +374,7 @@ func addExpiringCerts(t *testing.T, ctx *testCtx) []core.Certificate { Expires: ctx.fc.Now().AddDate(0, 0, 87), } - setupDBMap, err := sa.NewDbMap(vars.DBConnSAFullPerms, 0) + setupDBMap, err := sa.NewDbMap(vars.DBConnSAFullPerms, sa.DbSettings{}) test.AssertNotError(t, err, "sa.NewDbMap failed") err = setupDBMap.Insert(certA) test.AssertNotError(t, err, "Couldn't add certA") @@ -513,7 +513,7 @@ func TestCertIsRenewed(t *testing.T) { }, } - setupDBMap, err := sa.NewDbMap(vars.DBConnSAFullPerms, 0) + setupDBMap, err := sa.NewDbMap(vars.DBConnSAFullPerms, sa.DbSettings{}) if err != nil { t.Fatal(err) } @@ -605,7 +605,7 @@ func TestLifetimeOfACert(t *testing.T) { DER: certDerA, } - setupDBMap, err := sa.NewDbMap(vars.DBConnSAFullPerms, 0) + setupDBMap, err := sa.NewDbMap(vars.DBConnSAFullPerms, sa.DbSettings{}) test.AssertNotError(t, err, "sa.NewDbMap failed") err = setupDBMap.Insert(certA) test.AssertNotError(t, err, "unable to insert Certificate") @@ -706,7 +706,7 @@ func TestDontFindRevokedCert(t *testing.T) { DER: certDerA, } - setupDBMap, err := sa.NewDbMap(vars.DBConnSAFullPerms, 0) + setupDBMap, err := sa.NewDbMap(vars.DBConnSAFullPerms, sa.DbSettings{}) test.AssertNotError(t, err, "sa.NewDbMap failed") err = setupDBMap.Insert(certA) test.AssertNotError(t, err, "unable to insert Certificate") @@ -768,7 +768,7 @@ func TestDedupOnRegistration(t *testing.T) { DER: certDerB, } - setupDBMap, err := sa.NewDbMap(vars.DBConnSAFullPerms, 0) + setupDBMap, err := sa.NewDbMap(vars.DBConnSAFullPerms, sa.DbSettings{}) test.AssertNotError(t, err, "sa.NewDbMap failed") err = setupDBMap.Insert(certA) test.AssertNotError(t, err, "Couldn't add certA") @@ -812,7 +812,7 @@ type testCtx struct { func setup(t *testing.T, nagTimes []time.Duration) *testCtx { // We use the test_setup user (which has full permissions to everything) // because the SA we return is used for inserting data to set up the test. - dbMap, err := sa.NewDbMap(vars.DBConnSAFullPerms, 0) + dbMap, err := sa.NewDbMap(vars.DBConnSAFullPerms, sa.DbSettings{}) if err != nil { t.Fatalf("Couldn't connect the database: %s", err) } diff --git a/cmd/expired-authz-purger2/main.go b/cmd/expired-authz-purger2/main.go index 49b03a1c9..24f29ecf7 100644 --- a/cmd/expired-authz-purger2/main.go +++ b/cmd/expired-authz-purger2/main.go @@ -78,7 +78,13 @@ func main() { dbURL, err := c.ExpiredAuthzPurger2.DBConfig.URL() cmd.FailOnError(err, "Couldn't load DB URL") - dbMap, err := sa.NewDbMap(dbURL, c.ExpiredAuthzPurger2.DBConfig.MaxDBConns) + dbSettings := sa.DbSettings{ + MaxOpenConns: c.ExpiredAuthzPurger2.DBConfig.GetMaxOpenConns(), + MaxIdleConns: c.ExpiredAuthzPurger2.DBConfig.MaxIdleConns, + ConnMaxLifetime: c.ExpiredAuthzPurger2.DBConfig.ConnMaxLifetime.Duration, + ConnMaxIdleTime: c.ExpiredAuthzPurger2.DBConfig.ConnMaxIdleTime.Duration, + } + dbMap, err := sa.NewDbMap(dbURL, dbSettings) cmd.FailOnError(err, "Could not connect to database") for { diff --git a/cmd/id-exporter/main.go b/cmd/id-exporter/main.go index e835608b6..352e2ff89 100644 --- a/cmd/id-exporter/main.go +++ b/cmd/id-exporter/main.go @@ -176,7 +176,10 @@ func main() { dbURL, err := cfg.ContactExporter.DBConfig.URL() cmd.FailOnError(err, "Couldn't load DB URL") - dbMap, err := sa.NewDbMap(dbURL, 10) + dbSettings := sa.DbSettings{ + MaxOpenConns: 10, + } + dbMap, err := sa.NewDbMap(dbURL, dbSettings) cmd.FailOnError(err, "Could not connect to database") exporter := idExporter{ diff --git a/cmd/id-exporter/main_test.go b/cmd/id-exporter/main_test.go index d965dfd7d..dcb58cd68 100644 --- a/cmd/id-exporter/main_test.go +++ b/cmd/id-exporter/main_test.go @@ -371,7 +371,7 @@ func setup(t *testing.T) testCtx { log := blog.UseMock() // Using DBConnSAFullPerms to be able to insert registrations and certificates - dbMap, err := sa.NewDbMap(vars.DBConnSAFullPerms, 0) + dbMap, err := sa.NewDbMap(vars.DBConnSAFullPerms, sa.DbSettings{}) if err != nil { t.Fatalf("Couldn't connect the database: %s", err) } diff --git a/cmd/notify-mailer/main.go b/cmd/notify-mailer/main.go index e00541cb1..f4a25a78b 100644 --- a/cmd/notify-mailer/main.go +++ b/cmd/notify-mailer/main.go @@ -441,7 +441,10 @@ func main() { dbURL, err := cfg.NotifyMailer.DBConfig.URL() cmd.FailOnError(err, "Couldn't load DB URL") - dbMap, err := sa.NewDbMap(dbURL, 10) + dbSettings := sa.DbSettings{ + MaxOpenConns: 10, + } + dbMap, err := sa.NewDbMap(dbURL, dbSettings) cmd.FailOnError(err, "Could not connect to database") // Load email body diff --git a/cmd/ocsp-responder/main.go b/cmd/ocsp-responder/main.go index e0180c312..2dab45119 100644 --- a/cmd/ocsp-responder/main.go +++ b/cmd/ocsp-responder/main.go @@ -277,10 +277,16 @@ as generated by Boulder's ceremony command. dbConnect = config.Source } logger.Infof("Loading OCSP Database for CA Cert: %s", c.Common.IssuerCert) - dbMap, err := sa.NewDbMap(dbConnect, config.DBConfig.MaxDBConns) + dbSettings := sa.DbSettings{ + MaxOpenConns: config.DBConfig.GetMaxOpenConns(), + MaxIdleConns: config.DBConfig.MaxIdleConns, + ConnMaxLifetime: config.DBConfig.ConnMaxLifetime.Duration, + ConnMaxIdleTime: config.DBConfig.ConnMaxIdleTime.Duration, + } + dbMap, err := sa.NewDbMap(dbConnect, dbSettings) cmd.FailOnError(err, "Could not connect to database") sa.SetSQLDebug(dbMap, logger) - sa.InitDBMetrics(dbMap, stats) + sa.InitDBMetrics(dbMap, stats, dbSettings) issuerCerts := c.OCSPResponder.IssuerCerts if len(issuerCerts) == 0 { @@ -292,13 +298,13 @@ as generated by Boulder's ceremony command. source = &dbSource{dbMap, filter, c.OCSPResponder.Timeout.Duration, logger} - // Export the MaxDBConns + // Export the value for dbSettings.MaxOpenConns dbConnStat := prometheus.NewGauge(prometheus.GaugeOpts{ Name: "max_db_connections", Help: "Maximum number of DB connections allowed.", }) stats.MustRegister(dbConnStat) - dbConnStat.Set(float64(config.DBConfig.MaxDBConns)) + dbConnStat.Set(float64(dbSettings.MaxOpenConns)) } m := mux(stats, c.OCSPResponder.Path, source, logger) diff --git a/cmd/ocsp-updater/main.go b/cmd/ocsp-updater/main.go index d9c1b3294..b03a9b5e1 100644 --- a/cmd/ocsp-updater/main.go +++ b/cmd/ocsp-updater/main.go @@ -344,11 +344,17 @@ func main() { // Configure DB dbURL, err := conf.DBConfig.URL() cmd.FailOnError(err, "Couldn't load DB URL") - dbMap, err := sa.NewDbMap(dbURL, conf.DBConfig.MaxDBConns) + dbSettings := sa.DbSettings{ + MaxOpenConns: conf.DBConfig.GetMaxOpenConns(), + MaxIdleConns: conf.DBConfig.MaxIdleConns, + ConnMaxLifetime: conf.DBConfig.ConnMaxLifetime.Duration, + ConnMaxIdleTime: conf.DBConfig.ConnMaxIdleTime.Duration} + + dbMap, err := sa.NewDbMap(dbURL, dbSettings) cmd.FailOnError(err, "Could not connect to database") // Collect and periodically report DB metrics using the DBMap and prometheus stats. - sa.InitDBMetrics(dbMap, stats) + sa.InitDBMetrics(dbMap, stats, dbSettings) clk := cmd.Clock() diff --git a/cmd/ocsp-updater/main_test.go b/cmd/ocsp-updater/main_test.go index f99caaff7..a92e69368 100644 --- a/cmd/ocsp-updater/main_test.go +++ b/cmd/ocsp-updater/main_test.go @@ -42,7 +42,7 @@ func (ca *mockOCSP) GenerateOCSP(_ context.Context, req *capb.GenerateOCSPReques var log = blog.UseMock() func setup(t *testing.T) (*OCSPUpdater, core.StorageAuthority, *db.WrappedMap, clock.FakeClock, func()) { - dbMap, err := sa.NewDbMap(vars.DBConnSA, 0) + dbMap, err := sa.NewDbMap(vars.DBConnSA, sa.DbSettings{}) test.AssertNotError(t, err, "Failed to create dbMap") sa.SetSQLDebug(dbMap, log) diff --git a/ra/ra_test.go b/ra/ra_test.go index 8d1eecd57..90b681cca 100644 --- a/ra/ra_test.go +++ b/ra/ra_test.go @@ -283,7 +283,7 @@ func initAuthorities(t *testing.T) (*DummyValidationAuthority, *sa.SQLStorageAut // Set to some non-zero time. fc.Set(time.Date(2015, 3, 4, 5, 0, 0, 0, time.UTC)) - dbMap, err := sa.NewDbMap(vars.DBConnSA, 0) + dbMap, err := sa.NewDbMap(vars.DBConnSA, sa.DbSettings{}) if err != nil { t.Fatalf("Failed to create dbMap: %s", err) } diff --git a/sa/database.go b/sa/database.go index 3faf99678..bd18ce94c 100644 --- a/sa/database.go +++ b/sa/database.go @@ -3,6 +3,7 @@ package sa import ( "database/sql" "fmt" + "time" "github.com/go-gorp/gorp/v3" "github.com/go-sql-driver/mysql" @@ -12,11 +13,41 @@ import ( blog "github.com/letsencrypt/boulder/log" ) +// DbSettings contains settings for the database/sql driver. The zero +// value of each field means use the default setting from database/sql. +// ConnMaxIdleTime and ConnMaxLifetime should be set lower than their +// mariab counterparts interactive_timeout and wait_timeout. +type DbSettings struct { + // MaxOpenConns sets the maximum number of open connections to the + // database. If MaxIdleConns is greater than 0 and MaxOpenConns is + // less than MaxIdleConns, then MaxIdleConns will be reduced to + // match the new MaxOpenConns limit. If n < 0, then there is no + // limit on the number of open connections. + MaxOpenConns int + + // MaxIdleConns sets the maximum number of connections in the idle + // connection pool. If MaxOpenConns is greater than 0 but less than + // MaxIdleConns, then MaxIdleConns will be reduced to match the + // MaxOpenConns limit. If n < 0, no idle connections are retained. + MaxIdleConns int + + // ConnMaxLifetime sets the maximum amount of time a connection may + // be reused. Expired connections may be closed lazily before reuse. + // If d < 0, connections are not closed due to a connection's age. + ConnMaxLifetime time.Duration + + // ConnMaxIdleTime sets the maximum amount of time a connection may + // be idle. Expired connections may be closed lazily before reuse. + // If d < 0, connections are not closed due to a connection's idle + // time. + ConnMaxIdleTime time.Duration +} + // NewDbMap creates a wrapped root gorp mapping object. Create one of these for // each database schema you wish to map. Each DbMap contains a list of mapped // tables. It automatically maps the tables for the primary parts of Boulder // around the Storage Authority. -func NewDbMap(dbConnect string, maxOpenConns int) (*boulderDB.WrappedMap, error) { +func NewDbMap(dbConnect string, settings DbSettings) (*boulderDB.WrappedMap, error) { var err error var config *mysql.Config @@ -25,7 +56,7 @@ func NewDbMap(dbConnect string, maxOpenConns int) (*boulderDB.WrappedMap, error) return nil, err } - return NewDbMapFromConfig(config, maxOpenConns) + return NewDbMapFromConfig(config, settings) } // sqlOpen is used in the tests to check that the arguments are properly @@ -36,12 +67,35 @@ var sqlOpen = func(dbType, connectStr string) (*sql.DB, error) { // setMaxOpenConns is also used so that we can replace it for testing. var setMaxOpenConns = func(db *sql.DB, maxOpenConns int) { - db.SetMaxOpenConns(maxOpenConns) + if maxOpenConns != 0 { + db.SetMaxOpenConns(maxOpenConns) + } +} + +// setMaxIdleConns is also used so that we can replace it for testing. +var setMaxIdleConns = func(db *sql.DB, maxIdleConns int) { + if maxIdleConns != 0 { + db.SetMaxIdleConns(maxIdleConns) + } +} + +// setConnMaxLifetime is also used so that we can replace it for testing. +var setConnMaxLifetime = func(db *sql.DB, connMaxLifetime time.Duration) { + if connMaxLifetime != 0 { + db.SetConnMaxLifetime(connMaxLifetime) + } +} + +// setConnMaxIdleTime is also used so that we can replace it for testing. +var setConnMaxIdleTime = func(db *sql.DB, connMaxIdleTime time.Duration) { + if connMaxIdleTime != 0 { + db.SetConnMaxIdleTime(connMaxIdleTime) + } } // NewDbMapFromConfig functions similarly to NewDbMap, but it takes the // decomposed form of the connection string, a *mysql.Config. -func NewDbMapFromConfig(config *mysql.Config, maxOpenConns int) (*boulderDB.WrappedMap, error) { +func NewDbMapFromConfig(config *mysql.Config, settings DbSettings) (*boulderDB.WrappedMap, error) { adjustMySQLConfig(config) db, err := sqlOpen("mysql", config.FormatDSN()) @@ -51,7 +105,10 @@ func NewDbMapFromConfig(config *mysql.Config, maxOpenConns int) (*boulderDB.Wrap if err = db.Ping(); err != nil { return nil, err } - setMaxOpenConns(db, maxOpenConns) + setMaxOpenConns(db, settings.MaxOpenConns) + setMaxIdleConns(db, settings.MaxIdleConns) + setConnMaxLifetime(db, settings.ConnMaxLifetime) + setConnMaxIdleTime(db, settings.ConnMaxIdleTime) dialect := gorp.MySQLDialect{Engine: "InnoDB", Encoding: "UTF8"} dbmap := &gorp.DbMap{Db: db, Dialect: dialect, TypeConverter: BoulderTypeConverter{}} diff --git a/sa/database_test.go b/sa/database_test.go index 76da9d5f7..6a290eb66 100644 --- a/sa/database_test.go +++ b/sa/database_test.go @@ -5,35 +5,75 @@ import ( "errors" "strings" "testing" + "time" "github.com/letsencrypt/boulder/test" "github.com/letsencrypt/boulder/test/vars" ) func TestInvalidDSN(t *testing.T) { - _, err := NewDbMap("invalid", 0) + _, err := NewDbMap("invalid", DbSettings{}) test.AssertError(t, err, "DB connect string missing the slash separating the database name") } var errExpected = errors.New("expected") -func TestMaxOpenConns(t *testing.T) { +func TestDbSettings(t *testing.T) { + // TODO(#5248): Add a full db.mockWrappedMap to sa/database tests oldSetMaxOpenConns := setMaxOpenConns + oldSetMaxIdleConns := setMaxIdleConns + oldSetConnMaxLifetime := setConnMaxLifetime + oldSetConnMaxIdleTime := setConnMaxIdleTime defer func() { setMaxOpenConns = oldSetMaxOpenConns + setMaxIdleConns = oldSetMaxIdleConns + setConnMaxLifetime = oldSetConnMaxLifetime + setConnMaxIdleTime = oldSetConnMaxIdleTime }() + maxOpenConns := -1 + maxIdleConns := -1 + connMaxLifetime := time.Second * 1 + connMaxIdleTime := time.Second * 1 + setMaxOpenConns = func(db *sql.DB, m int) { maxOpenConns = m oldSetMaxOpenConns(db, maxOpenConns) } - _, err := NewDbMap("sa@tcp(boulder-mysql:3306)/boulder_sa_integration", 100) + setMaxIdleConns = func(db *sql.DB, m int) { + maxIdleConns = m + oldSetMaxIdleConns(db, maxIdleConns) + } + setConnMaxLifetime = func(db *sql.DB, c time.Duration) { + connMaxLifetime = c + oldSetConnMaxLifetime(db, connMaxLifetime) + } + setConnMaxIdleTime = func(db *sql.DB, c time.Duration) { + connMaxIdleTime = c + oldSetConnMaxIdleTime(db, connMaxIdleTime) + } + dbSettings := DbSettings{ + MaxOpenConns: 100, + MaxIdleConns: 100, + ConnMaxLifetime: 100, + ConnMaxIdleTime: 100, + } + _, err := NewDbMap("sa@tcp(boulder-mysql:3306)/boulder_sa_integration", dbSettings) if err != nil { t.Errorf("connecting to DB: %s", err) } if maxOpenConns != 100 { t.Errorf("maxOpenConns was not set: expected %d, got %d", 100, maxOpenConns) } + if maxIdleConns != 100 { + t.Errorf("maxIdleConns was not set: expected %d, got %d", 100, maxIdleConns) + } + if connMaxLifetime != 100 { + t.Errorf("connMaxLifetime was not set: expected %d, got %d", 100, connMaxLifetime) + } + if connMaxIdleTime != 100 { + t.Errorf("connMaxIdleTime was not set: expected %d, got %d", 100, connMaxIdleTime) + } } func TestNewDbMap(t *testing.T) { @@ -50,7 +90,7 @@ func TestNewDbMap(t *testing.T) { return nil, errExpected } - dbMap, err := NewDbMap(mysqlConnectURL, 0) + dbMap, err := NewDbMap(mysqlConnectURL, DbSettings{}) if err != errExpected { t.Errorf("got incorrect error. Got %v, expected %v", err, errExpected) } @@ -61,7 +101,7 @@ func TestNewDbMap(t *testing.T) { } func TestStrictness(t *testing.T) { - dbMap, err := NewDbMap(vars.DBConnSA, 1) + dbMap, err := NewDbMap(vars.DBConnSA, DbSettings{1, 0, 0, 0}) if err != nil { t.Fatal(err) } @@ -77,7 +117,7 @@ func TestStrictness(t *testing.T) { } func TestTimeouts(t *testing.T) { - dbMap, err := NewDbMap(vars.DBConnSA+"?readTimeout=1s", 1) + dbMap, err := NewDbMap(vars.DBConnSA+"?readTimeout=1s", DbSettings{1, 0, 0, 0}) if err != nil { t.Fatal("Error setting up DB:", err) } @@ -101,7 +141,7 @@ func TestTimeouts(t *testing.T) { // databases that have auto_increment columns use BIGINT for the data type. Our // data is too big for INT. func TestAutoIncrementSchema(t *testing.T) { - dbMap, err := NewDbMap(vars.DBInfoSchemaRoot, 1) + dbMap, err := NewDbMap(vars.DBInfoSchemaRoot, DbSettings{1, 0, 0, 0}) test.AssertNotError(t, err, "unexpected err making NewDbMap") var count int64 diff --git a/sa/metrics.go b/sa/metrics.go index 39be30b2e..da76cb750 100644 --- a/sa/metrics.go +++ b/sa/metrics.go @@ -11,6 +11,21 @@ var ( "Maximum number of DB connections allowed.", nil, nil) + maxIdleConns = prometheus.NewDesc( + "db_max_idle_connections", + "Maximum number of idle DB connections allowed.", + nil, nil) + + connMaxLifetime = prometheus.NewDesc( + "db_connection_max_lifetime", + "Maximum lifetime of DB connections allowed.", + nil, nil) + + connMaxIdleTime = prometheus.NewDesc( + "db_connection_max_idle_time", + "Maximum lifetime of idle DB connections allowed.", + nil, nil) + openConns = prometheus.NewDesc( "db_open_connections", "Number of established DB connections (in-use and idle).", @@ -48,7 +63,8 @@ var ( ) type dbMetricsCollector struct { - dbMap *db.WrappedMap + dbMap *db.WrappedMap + dbSettings DbSettings } // Describe is implemented with DescribeByCollect. That's possible because the @@ -77,6 +93,9 @@ func (dbc dbMetricsCollector) Collect(ch chan<- prometheus.Metric) { // Translate the DBMap's db.DBStats counter values into Prometheus metrics. dbMapStats := dbc.dbMap.Db.Stats() writeGauge(maxOpenConns, float64(dbMapStats.MaxOpenConnections)) + writeGauge(maxIdleConns, float64(dbc.dbSettings.MaxIdleConns)) + writeGauge(connMaxLifetime, float64(dbc.dbSettings.ConnMaxLifetime)) + writeGauge(connMaxIdleTime, float64(dbc.dbSettings.ConnMaxIdleTime)) writeGauge(openConns, float64(dbMapStats.OpenConnections)) writeGauge(inUse, float64(dbMapStats.InUse)) writeGauge(idle, float64(dbMapStats.Idle)) @@ -87,10 +106,10 @@ func (dbc dbMetricsCollector) Collect(ch chan<- prometheus.Metric) { } // InitDBMetrics will register a Collector that translates the provided dbMap's -// stats into Prometheus metrics on the fly. The stat values will be translated -// from the gorp dbMap's inner sql.DBMap's DBStats structure values. -func InitDBMetrics(dbMap *db.WrappedMap, stats prometheus.Registerer) { +// stats and DbSettings into Prometheus metrics on the fly. The stat values will +// be translated from the gorp dbMap's inner sql.DBMap's DBStats structure values +func InitDBMetrics(dbMap *db.WrappedMap, stats prometheus.Registerer, dbSettings DbSettings) { // Create a dbMetricsCollector and register it - dbc := dbMetricsCollector{dbMap} + dbc := dbMetricsCollector{dbMap, dbSettings} stats.MustRegister(dbc) } diff --git a/sa/sa_test.go b/sa/sa_test.go index f059e225d..841a661ac 100644 --- a/sa/sa_test.go +++ b/sa/sa_test.go @@ -44,7 +44,7 @@ var ctx = context.Background() func initSA(t *testing.T) (*SQLStorageAuthority, clock.FakeClock, func()) { features.Reset() - dbMap, err := NewDbMap(vars.DBConnSA, 0) + dbMap, err := NewDbMap(vars.DBConnSA, DbSettings{}) if err != nil { t.Fatalf("Failed to create dbMap: %s", err) } diff --git a/test/authz-filler/main.go b/test/authz-filler/main.go index 759861304..4920afd63 100644 --- a/test/authz-filler/main.go +++ b/test/authz-filler/main.go @@ -48,7 +48,10 @@ func main() { dbURL, err := config.Filler.DBConfig.URL() cmd.FailOnError(err, "Couldn't load DB URL") // Set max connections equal to parallelism. - dbMap, err := sa.NewDbMap(dbURL, int(config.Filler.Parallelism)) + dbSettings := sa.DbSettings{ + MaxOpenConns: int(config.Filler.Parallelism), + } + dbMap, err := sa.NewDbMap(dbURL, dbSettings) cmd.FailOnError(err, "Could not connect to database") dbMap.AddTableWithName(model{}, "pendingAuthorizations").SetKeys(false, "ID") diff --git a/test/config-next/admin-revoker.json b/test/config-next/admin-revoker.json index 8f672df2e..fe262e04d 100644 --- a/test/config-next/admin-revoker.json +++ b/test/config-next/admin-revoker.json @@ -1,7 +1,7 @@ { "revoker": { "dbConnectFile": "test/secrets/revoker_dburl", - "maxDBConns": 1, + "maxOpenConns": 1, "tls": { "caCertFile": "test/grpc-creds/minica.pem", "certFile": "test/grpc-creds/admin-revoker.boulder/cert.pem", diff --git a/test/config-next/bad-key-revoker.json b/test/config-next/bad-key-revoker.json index b6bd7385f..fac0dc17b 100644 --- a/test/config-next/bad-key-revoker.json +++ b/test/config-next/bad-key-revoker.json @@ -1,7 +1,7 @@ { "BadKeyRevoker": { "dbConnectFile": "test/secrets/badkeyrevoker_dburl", - "maxDBConns": 10, + "maxOpenConns": 10, "debugAddr": ":8020", "tls": { "caCertFile": "test/grpc-creds/minica.pem", diff --git a/test/config-next/cert-checker.json b/test/config-next/cert-checker.json index d6eff13d6..6fff9ead6 100644 --- a/test/config-next/cert-checker.json +++ b/test/config-next/cert-checker.json @@ -1,7 +1,7 @@ { "certChecker": { "dbConnectFile": "test/secrets/cert_checker_dburl", - "maxDBConns": 10, + "maxOpenConns": 10, "hostnamePolicyFile": "test/hostname-policy.yaml", "ignoredLints": [ "n_subject_common_name_included" diff --git a/test/config-next/contact-exporter.json b/test/config-next/contact-exporter.json index c99e2453b..fca93d52a 100644 --- a/test/config-next/contact-exporter.json +++ b/test/config-next/contact-exporter.json @@ -2,6 +2,6 @@ "contactExporter": { "passwordFile": "test/secrets/smtp_password", "dbConnectFile": "test/secrets/mailer_dburl", - "maxDBConns": 10 + "maxOpenConns": 10 } } diff --git a/test/config-next/expiration-mailer.json b/test/config-next/expiration-mailer.json index 713fdad0f..7a62a6265 100644 --- a/test/config-next/expiration-mailer.json +++ b/test/config-next/expiration-mailer.json @@ -6,7 +6,7 @@ "from": "Expiry bot ", "passwordFile": "test/secrets/smtp_password", "dbConnectFile": "test/secrets/mailer_dburl", - "maxDBConns": 10, + "maxOpenConns": 10, "nagTimes": ["24h", "72h", "168h", "336h"], "nagCheckInterval": "24h", "emailTemplate": "test/example-expiration-template", diff --git a/test/config-next/janitor.json b/test/config-next/janitor.json index ce3be5048..1a8760049 100644 --- a/test/config-next/janitor.json +++ b/test/config-next/janitor.json @@ -4,7 +4,7 @@ "stdoutLevel": 6 }, "dbConnectFile": "test/secrets/janitor_dburl", - "maxDBConns": 10, + "maxOpenConns": 10, "debugAddr": ":8014", "jobConfigs": [ { diff --git a/test/config-next/notify-mailer.json b/test/config-next/notify-mailer.json index 73864aeb5..1d17012e1 100644 --- a/test/config-next/notify-mailer.json +++ b/test/config-next/notify-mailer.json @@ -5,7 +5,7 @@ "username": "cert-manager@example.com", "passwordFile": "test/secrets/smtp_password", "dbConnectFile": "test/secrets/mailer_dburl", - "maxDBConns": 10 + "maxOpenConns": 10 }, "syslog": { "stdoutLevel": 7, diff --git a/test/config-next/ocsp-responder.json b/test/config-next/ocsp-responder.json index ae73fd49a..5db40d8fa 100644 --- a/test/config-next/ocsp-responder.json +++ b/test/config-next/ocsp-responder.json @@ -1,7 +1,7 @@ { "ocspResponder": { "dbConnectFile": "test/secrets/ocsp_responder_dburl", - "maxDBConns": 10, + "maxOpenConns": 10, "path": "/", "listenAddress": "0.0.0.0:4002", "issuerCerts": [ diff --git a/test/config-next/ocsp-updater.json b/test/config-next/ocsp-updater.json index c4d5cff7e..35a1bf5c8 100644 --- a/test/config-next/ocsp-updater.json +++ b/test/config-next/ocsp-updater.json @@ -1,7 +1,7 @@ { "ocspUpdater": { "dbConnectFile": "test/secrets/ocsp_updater_dburl", - "maxDBConns": 10, + "maxOpenConns": 10, "oldOCSPWindow": "2s", "oldOCSPBatchSize": 5000, "parallelGenerateOCSPRequests": 10, diff --git a/test/config-next/sa.json b/test/config-next/sa.json index 2760bb1ab..7b8f3619c 100644 --- a/test/config-next/sa.json +++ b/test/config-next/sa.json @@ -1,7 +1,7 @@ { "sa": { "dbConnectFile": "test/secrets/sa_dburl", - "maxDBConns": 100, + "maxOpenConns": 100, "ParallelismPerRPC": 20, "debugAddr": ":8003", "tls": {