X Tutup
Skip to content

Commit 42d70dd

Browse files
jshaDaniel McCarney
jsha
authored and
Daniel McCarney
committed
SA: Deprecate GetAuthorizationsPerf flag. (letsencrypt#4576)
In the process I tweaked a few variable names in GetAuthorizations2 to refer to just "authz" instead of "authz2" because it made things clearer, particularly in the case of authz2IDMap, which is a map of whether a given ID exists, not a map from authz's to IDs. Fixes letsencrypt#4564
1 parent 6ed62cf commit 42d70dd

File tree

4 files changed

+61
-77
lines changed

4 files changed

+61
-77
lines changed

‎features/featureflag_string.go‎

Lines changed: 17 additions & 17 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎features/features.go‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ const (
2626
EarlyOrderRateLimit
2727
FasterGetOrderForNames
2828
PrecertificateOCSP
29+
GetAuthorizationsPerf
2930

3031
// Currently in-use features
3132
// Check CAA and respect validationmethods parameter.
@@ -65,9 +66,6 @@ const (
6566
// StripDefaultSchemePort enables stripping of default scheme ports from HTTP
6667
// request Host headers
6768
StripDefaultSchemePort
68-
// GetAuthorizationsPerf enables a more performant GetAuthorizations2 query
69-
// at the SA.
70-
GetAuthorizationsPerf
7169
// StoreIssuerInfo enables storage of information identifying the issuer of
7270
// a certificate in the certificateStatus table.
7371
StoreIssuerInfo

‎sa/sa.go‎

Lines changed: 43 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import (
2121
corepb "github.com/letsencrypt/boulder/core/proto"
2222
"github.com/letsencrypt/boulder/db"
2323
berrors "github.com/letsencrypt/boulder/errors"
24-
"github.com/letsencrypt/boulder/features"
2524
bgrpc "github.com/letsencrypt/boulder/grpc"
2625
"github.com/letsencrypt/boulder/identifier"
2726
blog "github.com/letsencrypt/boulder/log"
@@ -1415,31 +1414,16 @@ func (ssa *SQLStorageAuthority) GetAuthorizations2(ctx context.Context, req *sap
14151414
params = append(params, n)
14161415
}
14171416
var query string
1418-
if features.Enabled(features.GetAuthorizationsPerf) {
1419-
query = fmt.Sprintf(
1420-
`SELECT %s FROM authz2
1417+
query = fmt.Sprintf(
1418+
`SELECT %s FROM authz2
14211419
WHERE registrationID = ? AND
14221420
status IN (?,?) AND
14231421
expires > ? AND
14241422
identifierType = ? AND
14251423
identifierValue IN (%s)`,
1426-
authz2Fields,
1427-
strings.Join(qmarks, ","),
1428-
)
1429-
} else {
1430-
query = fmt.Sprintf(
1431-
`SELECT %s FROM authz2
1432-
JOIN orderToAuthz2
1433-
ON id = authzID
1434-
WHERE registrationID = ? AND
1435-
status IN (?,?) AND
1436-
expires > ? AND
1437-
identifierType = ? AND
1438-
identifierValue IN (%s)`,
1439-
authz2Fields,
1440-
strings.Join(qmarks, ","),
1441-
)
1442-
}
1424+
authz2Fields,
1425+
strings.Join(qmarks, ","),
1426+
)
14431427
_, err := ssa.dbMap.Select(
14441428
&authzModels,
14451429
query,
@@ -1449,45 +1433,48 @@ func (ssa *SQLStorageAuthority) GetAuthorizations2(ctx context.Context, req *sap
14491433
return nil, err
14501434
}
14511435

1452-
authz2IDMap := map[int64]bool{}
1453-
// Once the old authorization storage format fallback is removed we don't need
1454-
// this length check as if there are none returned we can just return immediately.
1455-
if features.Enabled(features.GetAuthorizationsPerf) && len(authzModels) > 0 {
1456-
// Previously we used a JOIN on the orderToAuthz2 table in order to make sure
1457-
// we only returned authorizations created using the ACME v2 API. Each time an
1458-
// order is created a pivot row (order ID + authz ID) is added to the
1459-
// orderToAuthz2 table. If a large number of orders are created that all contain
1460-
// the same authorization, due to reuse, then the JOINd query would return a full
1461-
// authorization row for each entry in the orderToAuthz2 table with the authorization
1462-
// ID.
1463-
//
1464-
// Instead we now filter out these authorizations by doing a second query against
1465-
// the orderToAuthz2 table. Using this query still requires examining a large number
1466-
// of rows, but because we don't need to construct a temporary table for the JOIN
1467-
// and fill it with all the full authorization rows we should save resources.
1468-
var ids []interface{}
1469-
qmarks = make([]string, len(authzModels))
1470-
for i, am := range authzModels {
1471-
ids = append(ids, am.ID)
1472-
qmarks[i] = "?"
1473-
}
1474-
var authz2IDs []int64
1475-
_, err = ssa.dbMap.Select(
1476-
&authz2IDs,
1477-
fmt.Sprintf(`SELECT DISTINCT(authzID) FROM orderToAuthz2 WHERE authzID IN (%s)`, strings.Join(qmarks, ",")),
1478-
ids...,
1479-
)
1480-
if err != nil {
1481-
return nil, err
1482-
}
1483-
for _, id := range authz2IDs {
1484-
authz2IDMap[id] = true
1485-
}
1436+
if len(authzModels) == 0 {
1437+
return &sapb.Authorizations{}, nil
1438+
}
1439+
1440+
// Previously we used a JOIN on the orderToAuthz2 table in order to make sure
1441+
// we only returned authorizations created using the ACME v2 API. Each time an
1442+
// order is created a pivot row (order ID + authz ID) is added to the
1443+
// orderToAuthz2 table. If a large number of orders are created that all contain
1444+
// the same authorization, due to reuse, then the JOINd query would return a full
1445+
// authorization row for each entry in the orderToAuthz2 table with the authorization
1446+
// ID.
1447+
//
1448+
// Instead we now filter out these authorizations by doing a second query against
1449+
// the orderToAuthz2 table. Using this query still requires examining a large number
1450+
// of rows, but because we don't need to construct a temporary table for the JOIN
1451+
// and fill it with all the full authorization rows we should save resources.
1452+
var ids []interface{}
1453+
qmarks = make([]string, len(authzModels))
1454+
for i, am := range authzModels {
1455+
ids = append(ids, am.ID)
1456+
qmarks[i] = "?"
1457+
}
1458+
var authzIDs []int64
1459+
_, err = ssa.dbMap.Select(
1460+
&authzIDs,
1461+
fmt.Sprintf(`SELECT DISTINCT(authzID) FROM orderToAuthz2 WHERE authzID IN (%s)`, strings.Join(qmarks, ",")),
1462+
ids...,
1463+
)
1464+
if err != nil {
1465+
return nil, err
1466+
}
1467+
authzIDMap := map[int64]bool{}
1468+
for _, id := range authzIDs {
1469+
authzIDMap[id] = true
14861470
}
14871471

14881472
authzModelMap := make(map[string]authz2Model)
14891473
for _, am := range authzModels {
1490-
if _, present := authz2IDMap[am.ID]; features.Enabled(features.GetAuthorizationsPerf) && !present {
1474+
// Anything not found in the ID map wasn't in the pivot table, meaning it
1475+
// didn't correspond to an order, meaning it wasn't created with ACMEv2.
1476+
// Don't return it for ACMEv2 requests.
1477+
if _, present := authzIDMap[am.ID]; !present {
14911478
continue
14921479
}
14931480
if existing, present := authzModelMap[am.IdentifierValue]; !present ||

‎test/config-next/sa.json‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
"features": {
2727
"DeleteUnusedChallenges": true,
2828
"FasterGetOrderForNames": true,
29-
"GetAuthorizationsPerf": true,
3029
"StoreIssuerInfo": true
3130
}
3231
},

0 commit comments

Comments
 (0)
X Tutup