Add a MaxDBConns config parameter. (letsencrypt#1793)

jsha · jsha · commit b3bc3d8e4118 · 2016-05-09T14:21:15.000-07:00
diff --git a/cmd/admin-revoker/main.go b/cmd/admin-revoker/main.go
@@ -54,7 +54,7 @@ func setupContext(context *cli.Context) (rpc.RegistrationAuthorityClient, blog.L
 
 	dbURL, err := c.Revoker.DBConfig.URL()
 	cmd.FailOnError(err, "Couldn't load DB URL")
-	dbMap, err := sa.NewDbMap(dbURL)
+	dbMap, err := sa.NewDbMap(dbURL, c.Revoker.DBConfig.MaxDBConns)
 	cmd.FailOnError(err, "Couldn't setup database connection")
 
 	sac, err := rpc.NewStorageAuthorityClient(clientName, amqpConf, stats)
diff --git a/cmd/boulder-sa/main.go b/cmd/boulder-sa/main.go
@@ -23,7 +23,7 @@ func main() {
 
 		dbURL, err := saConf.DBConfig.URL()
 		cmd.FailOnError(err, "Couldn't load DB URL")
-		dbMap, err := sa.NewDbMap(dbURL)
+		dbMap, err := sa.NewDbMap(dbURL, saConf.DBConfig.MaxDBConns)
 		cmd.FailOnError(err, "Couldn't connect to SA database")
 
 		sai, err := sa.NewSQLStorageAuthority(dbMap, clock.Default(), logger)
diff --git a/cmd/cert-checker/main.go b/cmd/cert-checker/main.go
@@ -300,7 +300,7 @@ func main() {
 
 		saDbURL, err := config.CertChecker.DBConfig.URL()
 		cmd.FailOnError(err, "Couldn't load DB URL")
-		saDbMap, err := sa.NewDbMap(saDbURL)
+		saDbMap, err := sa.NewDbMap(saDbURL, config.CertChecker.DBConfig.MaxDBConns)
 		cmd.FailOnError(err, "Could not connect to database")
 
 		pa, err := policy.New(config.PA.Challenges)
diff --git a/cmd/cert-checker/main_test.go b/cmd/cert-checker/main_test.go
@@ -45,7 +45,7 @@ func init() {
 }
 
 func BenchmarkCheckCert(b *testing.B) {
-	saDbMap, err := sa.NewDbMap(vars.DBConnSA)
+	saDbMap, err := sa.NewDbMap(vars.DBConnSA, 0)
 	if err != nil {
 		fmt.Println("Couldn't connect to database")
 		return
@@ -81,7 +81,7 @@ func BenchmarkCheckCert(b *testing.B) {
 }
 
 func TestCheckCert(t *testing.T) {
-	saDbMap, err := sa.NewDbMap(vars.DBConnSA)
+	saDbMap, err := sa.NewDbMap(vars.DBConnSA, 0)
 	test.AssertNotError(t, err, "Couldn't connect to database")
 	saCleanup := test.ResetSATestDatabase(t)
 	defer func() {
@@ -179,7 +179,7 @@ func TestCheckCert(t *testing.T) {
 }
 
 func TestGetAndProcessCerts(t *testing.T) {
-	saDbMap, err := sa.NewDbMap(vars.DBConnSA)
+	saDbMap, err := sa.NewDbMap(vars.DBConnSA, 0)
 	test.AssertNotError(t, err, "Couldn't connect to database")
 	fc := clock.NewFake()
 
diff --git a/cmd/config.go b/cmd/config.go
@@ -259,6 +259,7 @@ type DBConfig struct {
 	DBConnect string
 	// A file containing a connect URL for the DB.
 	DBConnectFile string
+	MaxDBConns    int
 }
 
 // URL returns the DBConnect URL represented by this DBConfig object, either
diff --git a/cmd/expiration-mailer/main.go b/cmd/expiration-mailer/main.go
@@ -315,7 +315,7 @@ func main() {
 		// Configure DB
 		dbURL, err := c.Mailer.DBConfig.URL()
 		cmd.FailOnError(err, "Couldn't load DB URL")
-		dbMap, err := sa.NewDbMap(dbURL)
+		dbMap, err := sa.NewDbMap(dbURL, c.Mailer.DBConfig.MaxDBConns)
 		cmd.FailOnError(err, "Could not connect to database")
 
 		amqpConf := c.Mailer.AMQP
diff --git a/cmd/expiration-mailer/main_test.go b/cmd/expiration-mailer/main_test.go
@@ -341,7 +341,7 @@ func TestFindExpiringCertificates(t *testing.T) {
 		Expires: testCtx.fc.Now().AddDate(0, 0, 87),
 	}
 
-	setupDBMap, err := sa.NewDbMap(vars.DBConnSAFullPerms)
+	setupDBMap, err := sa.NewDbMap(vars.DBConnSAFullPerms, 0)
 	err = setupDBMap.Insert(certA)
 	test.AssertNotError(t, err, "Couldn't add certA")
 	err = setupDBMap.Insert(certB)
@@ -464,7 +464,7 @@ func TestCertIsRenewed(t *testing.T) {
 		},
 	}
 
-	setupDBMap, err := sa.NewDbMap(vars.DBConnSAFullPerms)
+	setupDBMap, err := sa.NewDbMap(vars.DBConnSAFullPerms, 0)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -565,7 +565,7 @@ func TestLifetimeOfACert(t *testing.T) {
 		Status: core.OCSPStatusGood,
 	}
 
-	setupDBMap, err := sa.NewDbMap(vars.DBConnSAFullPerms)
+	setupDBMap, err := sa.NewDbMap(vars.DBConnSAFullPerms, 0)
 	err = setupDBMap.Insert(certA)
 	test.AssertNotError(t, err, "unable to insert Certificate")
 	err = setupDBMap.Insert(certStatusA)
@@ -670,7 +670,7 @@ func TestDontFindRevokedCert(t *testing.T) {
 		Status: core.OCSPStatusRevoked,
 	}
 
-	setupDBMap, err := sa.NewDbMap(vars.DBConnSAFullPerms)
+	setupDBMap, err := sa.NewDbMap(vars.DBConnSAFullPerms, 0)
 	err = setupDBMap.Insert(certA)
 	test.AssertNotError(t, err, "unable to insert Certificate")
 	err = setupDBMap.Insert(certStatusA)
@@ -741,7 +741,7 @@ func TestDedupOnRegistration(t *testing.T) {
 		Status:                core.OCSPStatusGood,
 	}
 
-	setupDBMap, err := sa.NewDbMap(vars.DBConnSAFullPerms)
+	setupDBMap, err := sa.NewDbMap(vars.DBConnSAFullPerms, 0)
 	err = setupDBMap.Insert(certA)
 	test.AssertNotError(t, err, "Couldn't add certA")
 	err = setupDBMap.Insert(certB)
@@ -782,7 +782,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)
+	dbMap, err := sa.NewDbMap(vars.DBConnSAFullPerms, 0)
 	if err != nil {
 		t.Fatalf("Couldn't connect the database: %s", err)
 	}
diff --git a/cmd/external-cert-importer/main.go b/cmd/external-cert-importer/main.go
@@ -152,7 +152,7 @@ func main() {
 		// Configure DB
 		dbURL, err := c.PA.DBConfig.URL()
 		cmd.FailOnError(err, "Couldn't load DB URL")
-		dbMap, err := sa.NewDbMap(dbURL)
+		dbMap, err := sa.NewDbMap(dbURL, c.PA.DBConfig.MaxDBConns)
 		cmd.FailOnError(err, "Could not connect to database")
 
 		dbMap.AddTableWithName(core.ExternalCert{}, "externalCerts").SetKeys(false, "SHA1")
diff --git a/cmd/ocsp-responder/main.go b/cmd/ocsp-responder/main.go
@@ -145,7 +145,7 @@ func main() {
 
 		if url.Scheme == "mysql+tcp" {
 			logger.Info(fmt.Sprintf("Loading OCSP Database for CA Cert: %s", c.Common.IssuerCert))
-			dbMap, err := sa.NewDbMap(config.Source)
+			dbMap, err := sa.NewDbMap(config.Source, config.DBConfig.MaxDBConns)
 			cmd.FailOnError(err, "Could not connect to database")
 			sa.SetSQLDebug(dbMap, logger)
 			source, err = makeDBSource(dbMap, c.Common.IssuerCert, logger)
diff --git a/cmd/ocsp-updater/main.go b/cmd/ocsp-updater/main.go
@@ -571,7 +571,7 @@ func main() {
 		// Configure DB
 		dbURL, err := conf.DBConfig.URL()
 		cmd.FailOnError(err, "Couldn't load DB URL")
-		dbMap, err := sa.NewDbMap(dbURL)
+		dbMap, err := sa.NewDbMap(dbURL, conf.DBConfig.MaxDBConns)
 		cmd.FailOnError(err, "Could not connect to database")
 
 		cac, pubc, sac := setupClients(conf, stats)
diff --git a/cmd/ocsp-updater/main_test.go b/cmd/ocsp-updater/main_test.go
@@ -58,7 +58,7 @@ func (p *mockPub) SubmitToCT(_ context.Context, _ []byte) error {
 var log = blog.UseMock()
 
 func setup(t *testing.T) (*OCSPUpdater, core.StorageAuthority, *gorp.DbMap, clock.FakeClock, func()) {
-	dbMap, err := sa.NewDbMap(vars.DBConnSA)
+	dbMap, err := sa.NewDbMap(vars.DBConnSA, 0)
 	test.AssertNotError(t, err, "Failed to create dbMap")
 	sa.SetSQLDebug(dbMap, log)
 
diff --git a/ra/registration-authority_test.go b/ra/registration-authority_test.go
@@ -171,7 +171,7 @@ func initAuthorities(t *testing.T) (*DummyValidationAuthority, *sa.SQLStorageAut
 
 	fc := clock.NewFake()
 
-	dbMap, err := sa.NewDbMap(vars.DBConnSA)
+	dbMap, err := sa.NewDbMap(vars.DBConnSA, 0)
 	if err != nil {
 		t.Fatalf("Failed to create dbMap: %s", err)
 	}
diff --git a/sa/database.go b/sa/database.go
@@ -21,7 +21,7 @@ import (
 // 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) (*gorp.DbMap, error) {
+func NewDbMap(dbConnect string, maxOpenConns int) (*gorp.DbMap, error) {
 	var err error
 	var config *mysql.Config
 	if strings.HasPrefix(dbConnect, "mysql+tcp://") {
@@ -36,7 +36,7 @@ func NewDbMap(dbConnect string) (*gorp.DbMap, error) {
 		return nil, err
 	}
 
-	return NewDbMapFromConfig(config)
+	return NewDbMapFromConfig(config, maxOpenConns)
 }
 
 // sqlOpen is used in the tests to check that the arguments are properly
@@ -45,9 +45,14 @@ var sqlOpen = func(dbType, connectStr string) (*sql.DB, error) {
 	return sql.Open(dbType, connectStr)
 }
 
+// setMaxOpenConns is also used so that we can replace it for testing.
+var setMaxOpenConns = func(db *sql.DB, maxOpenConns int) {
+	db.SetMaxOpenConns(maxOpenConns)
+}
+
 // NewDbMapFromConfig functions similarly to NewDbMap, but it takes the
 // decomposed form of the connection string, a *mysql.Config.
-func NewDbMapFromConfig(config *mysql.Config) (*gorp.DbMap, error) {
+func NewDbMapFromConfig(config *mysql.Config, maxOpenConns int) (*gorp.DbMap, error) {
 	adjustMySQLConfig(config)
 
 	db, err := sqlOpen("mysql", config.FormatDSN())
@@ -57,6 +62,7 @@ func NewDbMapFromConfig(config *mysql.Config) (*gorp.DbMap, error) {
 	if err = db.Ping(); err != nil {
 		return nil, err
 	}
+	setMaxOpenConns(db, maxOpenConns)
 
 	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
@@ -14,12 +14,31 @@ import (
 )
 
 func TestInvalidDSN(t *testing.T) {
-	_, err := NewDbMap("invalid")
+	_, err := NewDbMap("invalid", 0)
 	test.AssertError(t, err, "DB connect string missing the slash separating the database name")
 }
 
 var errExpected = errors.New("expected")
 
+func TestMaxOpenConns(t *testing.T) {
+	oldSetMaxOpenConns := setMaxOpenConns
+	defer func() {
+		setMaxOpenConns = oldSetMaxOpenConns
+	}()
+	maxOpenConns := -1
+	setMaxOpenConns = func(db *sql.DB, m int) {
+		maxOpenConns = m
+		oldSetMaxOpenConns(db, maxOpenConns)
+	}
+	_, err := NewDbMap("sa@tcp(boulder-mysql:3306)/boulder_sa_integration", 100)
+	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)
+	}
+}
+
 func TestNewDbMap(t *testing.T) {
 	const mysqlConnectURL = "mysql+tcp://policy:password@boulder-mysql:3306/boulder_policy_integration?readTimeout=800ms&writeTimeout=800ms"
 	const expectedTransformed = "policy:password@tcp(boulder-mysql:3306)/boulder_policy_integration?clientFoundRows=true&parseTime=true&readTimeout=800ms&strict=true&writeTimeout=800ms"
@@ -35,7 +54,7 @@ func TestNewDbMap(t *testing.T) {
 		return nil, errExpected
 	}
 
-	dbMap, err := NewDbMap(mysqlConnectURL)
+	dbMap, err := NewDbMap(mysqlConnectURL, 0)
 	if err != errExpected {
 		t.Errorf("got incorrect error: %v", err)
 	}
diff --git a/sa/storage-authority_test.go b/sa/storage-authority_test.go
@@ -41,7 +41,7 @@ var ctx = context.Background()
 // initSA constructs a SQLStorageAuthority and a clean up function
 // that should be defer'ed to the end of the test.
 func initSA(t *testing.T) (*SQLStorageAuthority, clock.FakeClock, func()) {
-	dbMap, err := NewDbMap(vars.DBConnSA)
+	dbMap, err := NewDbMap(vars.DBConnSA, 0)
 	if err != nil {
 		t.Fatalf("Failed to create dbMap: %s", err)
 	}
diff --git a/test/boulder-config-next.json b/test/boulder-config-next.json
@@ -189,6 +189,7 @@
 
   "sa": {
     "dbConnectFile": "test/secrets/sa_dburl",
+    "maxDBConns": 10,
     "maxConcurrentRPCServerRequests": 16,
     "debugAddr": "localhost:8003",
     "amqp": {
@@ -235,6 +236,7 @@
 
   "revoker": {
     "dbConnectFile": "test/secrets/revoker_dburl",
+    "maxDBConns": 1,
     "amqp": {
       "serverURLFile": "test/secrets/amqp_url",
       "insecure": true,
@@ -261,6 +263,7 @@
 
   "ocspUpdater": {
     "dbConnectFile": "test/secrets/ocsp_updater_dburl",
+    "maxDBConns": 10,
     "newCertificateWindow": "1s",
     "oldOCSPWindow": "2s",
     "missingSCTWindow": "1m",
@@ -299,6 +302,7 @@
     "from": "Expiry bot <test@example.com>",
     "passwordFile": "test/secrets/smtp_password",
     "dbConnectFile": "test/secrets/mailer_dburl",
+    "maxDBConns": 10,
     "messageLimit": 0,
     "nagTimes": ["24h", "72h", "168h", "336h"],
     "nagCheckInterval": "24h",
@@ -347,7 +351,8 @@
   },
 
   "certChecker": {
-    "dbConnectFile": "test/secrets/cert_checker_dburl"
+    "dbConnectFile": "test/secrets/cert_checker_dburl",
+    "maxDBConns": 10
   },
 
   "subscriberAgreementURL": "http://boulder:4000/terms/v1",

Original file line number	Diff line number	Diff line change
`@@ -259,6 +259,7 @@ type DBConfig struct {`
`259`	`259`	`DBConnect string`
`260`	`260`	`// A file containing a connect URL for the DB.`
`261`	`261`	`DBConnectFile string`
	`262`	`+ MaxDBConns int`
`262`	`263`	`}`
`263`	`264`
`264`	`265`	`// URL returns the DBConnect URL represented by this DBConfig object, either`