Clean up NewOrderAndAuthzs

features/features.go

@@ -28,6 +28,7 @@ type Config struct {
 	MultiVAFullResults                bool
 	CertCheckerRequiresCorrespondence bool
 	ECDSAForAll                       bool
+	InsertAuthzsIndividually          bool
 
 	// ServeRenewalInfo exposes the renewalInfo endpoint in the directory and for
 	// GET requests. WARNING: This feature is a draft and highly unstable.
@@ -124,13 +125,6 @@ type Config struct {
 	//
 	// This flag should only be used in conjunction with UseKvLimitsForNewOrder.
 	DisableLegacyLimitWrites bool
-
-	// InsertAuthzsIndividually causes the SA's NewOrderAndAuthzs method to
-	// create each new authz one at a time, rather than using MultiInserter.
-	// Although this is expected to be a performance penalty, it is necessary to
-	// get the AUTO_INCREMENT ID of each new authz without relying on MariaDB's
-	// unique "INSERT ... RETURNING" functionality.
-	InsertAuthzsIndividually bool
 }
 
 var fMu = new(sync.RWMutex)

sa/sa_test.go

@@ -9,7 +9,6 @@ import (
 	"crypto/x509"
 	"database/sql"
 	"encoding/base64"
-	"encoding/hex"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -1063,12 +1062,12 @@ func TestFQDNSetsExists(t *testing.T) {
 	test.Assert(t, exists.Exists, "FQDN set does exist")
 }
 
-type queryRecorder struct {
+type execRecorder struct {
 	query string
 	args  []interface{}
 }
 
-func (e *queryRecorder) QueryContext(ctx context.Context, query string, args ...interface{}) (*sql.Rows, error) {
+func (e *execRecorder) ExecContext(ctx context.Context, query string, args ...interface{}) (sql.Result, error) {
 	e.query = query
 	e.args = args
 	return nil, nil
@@ -1154,7 +1153,7 @@ func TestAddIssuedNames(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.Name, func(t *testing.T) {
-			var e queryRecorder
+			var e execRecorder
 			err := addIssuedNames(
 				ctx,
 				&e,
@@ -1232,9 +1231,6 @@ func TestNewOrderAndAuthzs(t *testing.T) {
 	sa, _, cleanup := initSA(t)
 	defer cleanup()
 
-	features.Set(features.Config{InsertAuthzsIndividually: true})
-	defer features.Reset()
-
 	reg := createWorkingRegistration(t, sa)
 
 	// Insert two pre-existing authorizations to reference
@@ -1289,9 +1285,6 @@ func TestNewOrderAndAuthzs_NonNilInnerOrder(t *testing.T) {
 	sa, fc, cleanup := initSA(t)
 	defer cleanup()
 
-	features.Set(features.Config{InsertAuthzsIndividually: true})
-	defer features.Reset()
-
 	reg := createWorkingRegistration(t, sa)
 
 	expires := fc.Now().Add(2 * time.Hour)
@@ -1313,9 +1306,6 @@ func TestNewOrderAndAuthzs_MismatchedRegID(t *testing.T) {
 	sa, _, cleanup := initSA(t)
 	defer cleanup()
 
-	features.Set(features.Config{InsertAuthzsIndividually: true})
-	defer features.Reset()
-
 	_, err := sa.NewOrderAndAuthzs(context.Background(), &sapb.NewOrderAndAuthzsRequest{
 		NewOrder: &sapb.NewOrderRequest{
 			RegistrationID: 1,
@@ -1334,9 +1324,6 @@ func TestNewOrderAndAuthzs_NewAuthzExpectedFields(t *testing.T) {
 	sa, fc, cleanup := initSA(t)
 	defer cleanup()
 
-	features.Set(features.Config{InsertAuthzsIndividually: true})
-	defer features.Reset()
-
 	reg := createWorkingRegistration(t, sa)
 	expires := fc.Now().Add(time.Hour)
 	domain := "a.com"
@@ -1385,55 +1372,6 @@ func TestNewOrderAndAuthzs_NewAuthzExpectedFields(t *testing.T) {
 	test.AssertBoxedNil(t, am.ValidationRecord, "am.ValidationRecord should be nil")
 }
 
-func BenchmarkNewOrderAndAuthzs(b *testing.B) {
-	for _, flag := range []bool{false, true} {
-		for _, numIdents := range []int{1, 2, 5, 10, 20, 50, 100} {
-			b.Run(fmt.Sprintf("%t/%d", flag, numIdents), func(b *testing.B) {
-				sa, _, cleanup := initSA(b)
-				defer cleanup()
-
-				if flag {
-					features.Set(features.Config{InsertAuthzsIndividually: true})
-					defer features.Reset()
-				}
-
-				reg := createWorkingRegistration(b, sa)
-
-				dnsNames := make([]string, 0, numIdents)
-				newAuthzs := make([]*sapb.NewAuthzRequest, 0, numIdents)
-				for range numIdents {
-					var nameBytes [3]byte
-					_, _ = rand.Read(nameBytes[:])
-					name := fmt.Sprintf("%s.example.com", hex.EncodeToString(nameBytes[:]))
-
-					dnsNames = append(dnsNames, name)
-					newAuthzs = append(newAuthzs, &sapb.NewAuthzRequest{
-						RegistrationID: reg.Id,
-						Identifier:     identifier.NewDNS(name).AsProto(),
-						ChallengeTypes: []string{string(core.ChallengeTypeDNS01)},
-						Token:          core.NewToken(),
-						Expires:        timestamppb.New(sa.clk.Now().Add(24 * time.Hour)),
-					})
-				}
-
-				b.ResetTimer()
-
-				_, err := sa.NewOrderAndAuthzs(context.Background(), &sapb.NewOrderAndAuthzsRequest{
-					NewOrder: &sapb.NewOrderRequest{
-						RegistrationID: reg.Id,
-						Expires:        timestamppb.New(sa.clk.Now().Add(24 * time.Hour)),
-						DnsNames:       dnsNames,
-					},
-					NewAuthzs: newAuthzs,
-				})
-				if err != nil {
-					b.Error(err)
-				}
-			})
-		}
-	}
-}
-
 func TestSetOrderProcessing(t *testing.T) {
 	sa, fc, cleanup := initSA(t)
 	defer cleanup()

test/config-next/sa.json

@@ -51,8 +51,7 @@
 		"features": {
 			"MultipleCertificateProfiles": true,
 			"TrackReplacementCertificatesARI": true,
-			"DisableLegacyLimitWrites": true,
-			"InsertAuthzsIndividually": true
+			"DisableLegacyLimitWrites": true
 		}
 	},
 	"syslog": {

test/config/sa.json

@@ -47,7 +47,9 @@
 				}
 			}
 		},
-		"features": {}
+		"features": {
+			"InsertAuthzsIndividually": true
+		}
 	},
 	"syslog": {
 		"stdoutlevel": 6,