Revert "SA: improve performance of GetOrderForNames. (#4326)" (#4328)

This reverts commit 9fa360769e.

This commit can cause "gorp: multiple rows returned for: ..." under certain situations.

See #4329 for details of followup.

features/featureflag_string.go

@@ -29,12 +29,11 @@ func _() {
 	_ = x[RemoveWFE2AccountID-18]
 	_ = x[CheckRenewalFirst-19]
 	_ = x[MandatoryPOSTAsGET-20]
-	_ = x[FasterGetOrderForNames-21]
 }
 
-const _FeatureFlag_name = "unusedPerformValidationRPCACME13KeyRolloverSimplifiedVAHTTPTLSSNIRevalidationAllowRenewalFirstRLSetIssuedNamesRenewalBitFasterRateLimitProbeCTLogsRevokeAtRACAAValidationMethodsCAAAccountURIHeadNonceStatusOKNewAuthorizationSchemaDisableAuthz2OrdersEarlyOrderRateLimitEnforceMultiVAMultiVAFullResultsRemoveWFE2AccountIDCheckRenewalFirstMandatoryPOSTAsGETFasterGetOrderForNames"
+const _FeatureFlag_name = "unusedPerformValidationRPCACME13KeyRolloverSimplifiedVAHTTPTLSSNIRevalidationAllowRenewalFirstRLSetIssuedNamesRenewalBitFasterRateLimitProbeCTLogsRevokeAtRACAAValidationMethodsCAAAccountURIHeadNonceStatusOKNewAuthorizationSchemaDisableAuthz2OrdersEarlyOrderRateLimitEnforceMultiVAMultiVAFullResultsRemoveWFE2AccountIDCheckRenewalFirstMandatoryPOSTAsGET"
 
-var _FeatureFlag_index = [...]uint16{0, 6, 26, 43, 59, 77, 96, 120, 135, 146, 156, 176, 189, 206, 228, 247, 266, 280, 298, 317, 334, 352, 374}
+var _FeatureFlag_index = [...]uint16{0, 6, 26, 43, 59, 77, 96, 120, 135, 146, 156, 176, 189, 206, 228, 247, 266, 280, 298, 317, 334, 352}
 
 func (i FeatureFlag) String() string {
 	if i < 0 || i >= FeatureFlag(len(_FeatureFlag_index)-1) {

features/features.go

@@ -57,8 +57,6 @@ const (
 	// MandatoryPOSTAsGET forbids legacy unauthenticated GET requests for ACME
 	// resources.
 	MandatoryPOSTAsGET
-	// Use an optimized query for GetOrderForNames.
-	FasterGetOrderForNames
 )
 
 // List of features and their default value, protected by fMu
@@ -84,7 +82,6 @@ var features = map[FeatureFlag]bool{
 	CheckRenewalFirst:        false,
 	MandatoryPOSTAsGET:       false,
 	DisableAuthz2Orders:      false,
-	FasterGetOrderForNames:   false,
 }
 
 var fMu = new(sync.RWMutex)

sa/sa.go

@@ -1812,58 +1812,29 @@ func (ssa *SQLStorageAuthority) GetOrderForNames(
 	// Hash the names requested for lookup in the orderFqdnSets table
 	fqdnHash := hashNames(req.Names)
 
-	// Find a possibly-suitable order. We don't include the account ID or order
-	// status in this query because there's no index that includes those, so
-	// including them could require the DB to scan extra rows.
-	// Instead, we select one unexpired order that matches the fqdnSet. If
-	// that order doesn't match the account ID or status we need, just return
-	// nothing. We use `ORDER BY expires ASC` because the index on
-	// (setHash, expires) is in ASC order. DESC would be slightly nicer from a
-	// user experience perspective but would be slow when there are many entries
-	// to sort.
-	// This approach works fine because in most cases there's only one account
-	// issuing for a given name. If there are other accounts issuing for the same
-	// name, it just means order reuse happens less often.
-	var result struct {
-		OrderID        int64
-		RegistrationID int64
-	}
-	var err error
-	if features.Enabled(features.FasterGetOrderForNames) {
-		err = ssa.dbMap.WithContext(ctx).SelectOne(&result, `
-					SELECT orderID, registrationID
-					FROM orderFqdnSets
-					WHERE setHash = ?
-					AND expires > ?
-					ORDER BY expires DESC
-					LIMIT 1`,
-			fqdnHash, ssa.clk.Now())
-	} else {
-		err = ssa.dbMap.WithContext(ctx).SelectOne(&result, `
-					SELECT orderID, registrationID
-					FROM orderFqdnSets
-					WHERE setHash = ?
-					AND registrationID = ?
-					AND expires > ?`,
-			fqdnHash, *req.AcctID, ssa.clk.Now())
-	}
+	var orderID int64
+	err := ssa.dbMap.WithContext(ctx).SelectOne(&orderID, `
+	SELECT orderID
+	FROM orderFqdnSets
+	WHERE setHash = ?
+	AND registrationID = ?
+	AND expires > ?`,
+		fqdnHash, *req.AcctID, ssa.clk.Now())
 
+	// There isn't an unexpired order for the provided AcctID that has the
+	// fqdnHash requested.
 	if err == sql.ErrNoRows {
 		return nil, berrors.NotFoundError("no order matching request found")
 	} else if err != nil {
+		// An unexpected error occurred
 		return nil, err
 	}
 
-	if result.RegistrationID != *req.AcctID {
-		return nil, berrors.NotFoundError("no order matching request found")
-	}
-
 	// Get the order
-	order, err := ssa.GetOrder(ctx, &sapb.OrderRequest{Id: &result.OrderID, UseV2Authorizations: req.UseV2Authorizations})
+	order, err := ssa.GetOrder(ctx, &sapb.OrderRequest{Id: &orderID, UseV2Authorizations: req.UseV2Authorizations})
 	if err != nil {
 		return nil, err
 	}
-
 	// Only return a pending or ready order
 	if *order.Status != string(core.StatusPending) &&
 		*order.Status != string(core.StatusReady) {

test/config-next/sa.json

@@ -24,7 +24,6 @@
       ]
     },
     "features": {
-      "FasterGetOrderForNames": true
     }
   },