sa: truncate all timestamps to seconds (#7519)

As described in #7075, go-sql-driver/mysql v1.5.0 truncates timestamps
to microseconds, while v1.6.0 and above does not. That means upon
upgrading to v1.6.0, timestamps are written to the database with a
resolution of nanoseconds, and SELECT statements also use a resolution
of nanoseconds. We believe this is the cause of performance problems
we observed when upgrading to v1.6.0 and above.

To fix that, apply rounding in the application code. Rather than
just rounding to microseconds, round to seconds since that is the
resolution we care about.  Using seconds rather than microseconds
may also allow some of our indexes to grow more slowly over time.

Note: this omits truncating some timestamps in CRL shard calculations,
since truncating those resulted in test failures that I'll follow up
on separately.

Author: jsha

cmd/bad-key-revoker/main.go

@@ -141,7 +141,7 @@ func (bkr *badKeyRevoker) findUnrevoked(ctx context.Context, unchecked unchecked
 			"SELECT id, certSerial FROM keyHashToSerial WHERE keyHash = ? AND id > ? AND certNotAfter > ? ORDER BY id LIMIT ?",
 			unchecked.KeyHash,
 			initialID,
-			bkr.clk.Now(),
+			bkr.clk.Now().Truncate(time.Second),
 			bkr.serialBatchSize,
 		)
 		if err != nil {

docs/CONTRIBUTING.md

@@ -324,6 +324,13 @@ value. The `core.IsAnyNilOrZero` function can check these cases.
 Senders must check that timestamps are non-zero before sending them. Receivers
 must check that timestamps are non-zero before accepting them.
 
+# Rounding time in DB
+
+All times that we write to the database are truncated to one second's worth of
+precision. This reduces the size of indexes that include timestamps, and makes
+querying them more efficient. The Storage Authority (SA) is responsible for this
+truncation, and performs it for SELECT queries as well as INSERT and UPDATE.
+
 # Release Process
 
 The current Boulder release process is described in

sa/sa.go

@@ -108,7 +108,7 @@ func (ssa *SQLStorageAuthority) NewRegistration(ctx context.Context, req *corepb
 		return nil, err
 	}
 
-	reg.CreatedAt = ssa.clk.Now()
+	reg.CreatedAt = ssa.clk.Now().Truncate(time.Second)
 
 	err = ssa.dbMap.Insert(ctx, reg)
 	if err != nil {
@@ -141,6 +141,10 @@ func (ssa *SQLStorageAuthority) UpdateRegistration(ctx context.Context, req *cor
 		return nil, err
 	}
 
+	// The CreatedAt field shouldn't change from the original, so we copy it straight through.
+	// This also ensures that it's already truncated to second (which happened on creation).
+	update.CreatedAt = curr.CreatedAt
+
 	// Copy the existing registration model's LockCol to the new updated
 	// registration model's LockCol
 	update.LockCol = curr.LockCol
@@ -169,8 +173,8 @@ func (ssa *SQLStorageAuthority) AddSerial(ctx context.Context, req *sapb.AddSeri
 	err := ssa.dbMap.Insert(ctx, &recordedSerialModel{
 		Serial:         req.Serial,
 		RegistrationID: req.RegID,
-		Created:        req.Created.AsTime(),
-		Expires:        req.Expires.AsTime(),
+		Created:        req.Created.AsTime().Truncate(time.Second),
+		Expires:        req.Expires.AsTime().Truncate(time.Second),
 	})
 	if err != nil {
 		return nil, err
@@ -223,7 +227,7 @@ func (ssa *SQLStorageAuthority) AddPrecertificate(ctx context.Context, req *sapb
 		Serial:         serialHex,
 		RegistrationID: req.RegID,
 		DER:            req.Der,
-		Issued:         req.Issued.AsTime(),
+		Issued:         req.Issued.AsTime().Truncate(time.Second),
 		Expires:        parsed.NotAfter,
 	}
 
@@ -252,13 +256,15 @@ func (ssa *SQLStorageAuthority) AddPrecertificate(ctx context.Context, req *sapb
 		cs := &core.CertificateStatus{
 			Serial:                serialHex,
 			Status:                status,
-			OCSPLastUpdated:       ssa.clk.Now(),
+			OCSPLastUpdated:       ssa.clk.Now().Truncate(time.Second),
 			RevokedDate:           time.Time{},
 			RevokedReason:         0,
 			LastExpirationNagSent: time.Time{},
-			NotAfter:              parsed.NotAfter,
-			IsExpired:             false,
-			IssuerNameID:          req.IssuerNameID,
+			// No need to truncate because it's already truncated to encode
+			// per https://datatracker.ietf.org/doc/html/rfc5280#section-4.1.2.5.1
+			NotAfter:     parsed.NotAfter,
+			IsExpired:    false,
+			IssuerNameID: req.IssuerNameID,
 		}
 		err = ssa.dbMap.Insert(ctx, cs)
 		if err != nil {
@@ -317,7 +323,7 @@ func (ssa *SQLStorageAuthority) AddCertificate(ctx context.Context, req *sapb.Ad
 		Serial:         serial,
 		Digest:         digest,
 		DER:            req.Der,
-		Issued:         req.Issued.AsTime(),
+		Issued:         req.Issued.AsTime().Truncate(time.Second),
 		Expires:        parsedCertificate.NotAfter,
 	}
 
@@ -476,13 +482,15 @@ func (ssa *SQLStorageAuthority) NewOrderAndAuthzs(ctx context.Context, req *sapb
 				if err != nil {
 					return nil, err
 				}
+				// These parameters correspond to the fields listed in `authzFields`, as used in the
+				// `db.NewMultiInserter` call above, and occur in the same order.
 				err = inserter.Add([]interface{}{
 					am.ID,
 					am.IdentifierType,
 					am.IdentifierValue,
 					am.RegistrationID,
 					statusToUint[core.StatusPending],
-					am.Expires,
+					am.Expires.Truncate(time.Second),
 					am.Challenges,
 					nil,
 					nil,
@@ -503,11 +511,12 @@ func (ssa *SQLStorageAuthority) NewOrderAndAuthzs(ctx context.Context, req *sapb
 		// Second, insert the new order.
 		var orderID int64
 		var err error
-		created := ssa.clk.Now()
+		created := ssa.clk.Now().Truncate(time.Second)
+		expires := req.NewOrder.Expires.AsTime().Truncate(time.Second)
 		if features.Get().MultipleCertificateProfiles {
 			omv2 := orderModelv2{
 				RegistrationID:         req.NewOrder.RegistrationID,
-				Expires:                req.NewOrder.Expires.AsTime(),
+				Expires:                expires,
 				Created:                created,
 				CertificateProfileName: req.NewOrder.CertificateProfileName,
 			}
@@ -516,7 +525,7 @@ func (ssa *SQLStorageAuthority) NewOrderAndAuthzs(ctx context.Context, req *sapb
 		} else {
 			omv1 := orderModelv1{
 				RegistrationID: req.NewOrder.RegistrationID,
-				Expires:        req.NewOrder.Expires.AsTime(),
+				Expires:        expires,
 				Created:        created,
 			}
 			err = tx.Insert(ctx, &omv1)
@@ -549,19 +558,25 @@ func (ssa *SQLStorageAuthority) NewOrderAndAuthzs(ctx context.Context, req *sapb
 		}
 
 		// Fourth, insert the FQDNSet entry for the order.
-		err = addOrderFQDNSet(ctx, tx, req.NewOrder.Names, orderID, req.NewOrder.RegistrationID, req.NewOrder.Expires.AsTime())
+		err = addOrderFQDNSet(ctx,
+			tx,
+			req.NewOrder.Names,
+			orderID,
+			req.NewOrder.RegistrationID,
+			expires,
+		)
 		if err != nil {
 			return nil, err
 		}
 
-		// Finally, build the overall Order PB.
+		// Finally, build the overall Order PB to return.
 		res := &corepb.Order{
 			// ID and Created were auto-populated on the order model when it was inserted.
 			Id:      orderID,
 			Created: timestamppb.New(created),
 			// These are carried over from the original request unchanged.
 			RegistrationID: req.NewOrder.RegistrationID,
-			Expires:        req.NewOrder.Expires,
+			Expires:        timestamppb.New(expires),
 			Names:          req.NewOrder.Names,
 			// Have to combine the already-associated and newly-reacted authzs.
 			V2Authorizations: append(req.NewOrder.V2Authorizations, newAuthzIDs...),
@@ -576,7 +591,12 @@ func (ssa *SQLStorageAuthority) NewOrderAndAuthzs(ctx context.Context, req *sapb
 		if req.NewOrder.ReplacesSerial != "" {
 			// Update the replacementOrders table to indicate that this order
 			// replaces the provided certificate serial.
-			err := addReplacementOrder(ctx, tx, req.NewOrder.ReplacesSerial, orderID, req.NewOrder.Expires.AsTime())
+			err := addReplacementOrder(ctx,
+				tx,
+				req.NewOrder.ReplacesSerial,
+				orderID,
+				req.NewOrder.Expires.AsTime().Truncate(time.Second),
+			)
 			if err != nil {
 				return nil, err
 			}
@@ -789,7 +809,7 @@ func (ssa *SQLStorageAuthority) FinalizeAuthorization2(ctx context.Context, req
 	// database attemptedAt field Null instead of 1970-01-01 00:00:00.
 	var attemptedTime *time.Time
 	if !core.IsAnyNilOrZero(req.AttemptedAt) {
-		val := req.AttemptedAt.AsTime()
+		val := req.AttemptedAt.AsTime().Truncate(time.Second)
 		attemptedTime = &val
 	}
 	params := map[string]interface{}{
@@ -799,7 +819,7 @@ func (ssa *SQLStorageAuthority) FinalizeAuthorization2(ctx context.Context, req
 		"validationRecord": vrJSON,
 		"id":               req.Id,
 		"pending":          statusUint(core.StatusPending),
-		"expires":          req.Expires.AsTime(),
+		"expires":          req.Expires.AsTime().Truncate(time.Second),
 		// if req.ValidationError is nil veJSON should also be nil
 		// which should result in a NULL field
 		"validationError": veJSON,
@@ -867,7 +887,7 @@ func (ssa *SQLStorageAuthority) RevokeCertificate(ctx context.Context, req *sapb
 	}
 
 	_, overallError := db.WithTransaction(ctx, ssa.dbMap, func(tx db.Executor) (interface{}, error) {
-		revokedDate := req.Date.AsTime()
+		revokedDate := req.Date.AsTime().Truncate(time.Second)
 
 		res, err := tx.ExecContext(ctx,
 			`UPDATE certificateStatus SET
@@ -924,8 +944,8 @@ func (ssa *SQLStorageAuthority) UpdateRevokedCertificate(ctx context.Context, re
 	}
 
 	_, overallError := db.WithTransaction(ctx, ssa.dbMap, func(tx db.Executor) (interface{}, error) {
-		thisUpdate := req.Date.AsTime()
-		revokedDate := req.Backdate.AsTime()
+		thisUpdate := req.Date.AsTime().Truncate(time.Second)
+		revokedDate := req.Backdate.AsTime().Truncate(time.Second)
 
 		res, err := tx.ExecContext(ctx,
 			`UPDATE certificateStatus SET
@@ -1007,7 +1027,7 @@ func (ssa *SQLStorageAuthority) AddBlockedKey(ctx context.Context, req *sapb.Add
 	cols, qs := blockedKeysColumns, "?, ?, ?, ?"
 	vals := []interface{}{
 		req.KeyHash,
-		req.Added.AsTime(),
+		req.Added.AsTime().Truncate(time.Second),
 		sourceInt,
 		req.Comment,
 	}
@@ -1232,7 +1252,10 @@ func (ssa *SQLStorageAuthority) leaseSpecificCRLShard(ctx context.Context, req *
 
 // UpdateCRLShard updates the thisUpdate and nextUpdate timestamps of a CRL
 // shard. It rejects the update if it would cause the thisUpdate timestamp to
-// move backwards. It does *not* reject the update if the shard is no longer
+// move backwards, but if thisUpdate would stay the same (for instance, multiple
+// CRL generations within a single second), it will succeed.
+//
+// It does *not* reject the update if the shard is no longer
 // leased: although this would be unexpected (because the lease timestamp should
 // be the same as the crl-updater's context expiration), it's not inherently a
 // sign of an update that should be skipped. It does reject the update if the
@@ -1247,24 +1270,25 @@ func (ssa *SQLStorageAuthority) UpdateCRLShard(ctx context.Context, req *sapb.Up
 	// Only set the nextUpdate if it's actually present in the request message.
 	var nextUpdate *time.Time
 	if req.NextUpdate != nil {
-		nut := req.NextUpdate.AsTime()
+		nut := req.NextUpdate.AsTime().Truncate(time.Second)
 		nextUpdate = &nut
 	}
 
 	_, err := db.WithTransaction(ctx, ssa.dbMap, func(tx db.Executor) (interface{}, error) {
+		thisUpdate := req.ThisUpdate.AsTime().Truncate(time.Second)
 		res, err := tx.ExecContext(ctx,
 			`UPDATE crlShards
 				SET thisUpdate = ?, nextUpdate = ?, leasedUntil = ?
 				WHERE issuerID = ?
 				AND idx = ?
-				AND (thisUpdate is NULL OR thisUpdate < ?)
+				AND (thisUpdate is NULL OR thisUpdate <= ?)
 				LIMIT 1`,
-			req.ThisUpdate.AsTime(),
+			thisUpdate,
 			nextUpdate,
-			req.ThisUpdate.AsTime(),
+			thisUpdate,
 			req.IssuerNameID,
 			req.ShardIdx,
-			req.ThisUpdate.AsTime(),
+			thisUpdate,
 		)
 		if err != nil {
 			return nil, err
@@ -1275,7 +1299,7 @@ func (ssa *SQLStorageAuthority) UpdateCRLShard(ctx context.Context, req *sapb.Up
 			return nil, err
 		}
 		if rowsAffected == 0 {
-			return nil, fmt.Errorf("unable to update shard %d for issuer %d", req.ShardIdx, req.IssuerNameID)
+			return nil, fmt.Errorf("unable to update shard %d for issuer %d; possibly because shard exists", req.ShardIdx, req.IssuerNameID)
 		}
 		if rowsAffected != 1 {
 			return nil, errors.New("update affected unexpected number of rows")

sa/sa_test.go

@@ -4012,7 +4012,7 @@ func TestUpdateCRLShard(t *testing.T) {
 		`SELECT thisUpdate FROM crlShards WHERE issuerID = 1 AND idx = 0 LIMIT 1`,
 	)
 	test.AssertNotError(t, err, "getting updated thisUpdate timestamp")
-	test.Assert(t, crlModel.ThisUpdate.Equal(thisUpdate), "checking updated thisUpdate timestamp")
+	test.AssertEquals(t, *crlModel.ThisUpdate, thisUpdate)
 
 	// Updating an unleased shard should work.
 	_, err = sa.UpdateCRLShard(