Skip to content

Conversation

aarongable
Copy link
Contributor

@aarongable aarongable commented Oct 2, 2025

Give the revokedCertificates table a non-unique index on the serial column. This exactly matches the existing index on the certificateStatus table.

We did not include this index initially because we were optimizing this table for the crl-updater's crawl-by-shard-and-expiration behavior, but this index is necessary to look up if a certificate has already been revoked when computing ARI windows and when processing revocation requests.

Part of #8322

IN-11835 tracks the corresponding production schema changes

@aarongable aarongable requested a review from a team as a code owner October 2, 2025 22:31
@aarongable aarongable requested a review from jsha October 2, 2025 22:31
Copy link
Contributor

github-actions bot commented Oct 2, 2025

@aarongable, this PR appears to contain configuration and/or SQL schema changes. Please ensure that a corresponding deployment ticket has been filed with the new values.

-- +migrate Up
-- SQL in section 'Up' is executed when this migration is applied

ALTER TABLE `revokedCertificates` ADD KEY `serial` (`serial`);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: I'd expected to see ADD UNIQUE KEY. But, if I'm remembering correctly, with partitioning we can't use UNIQUE KEYs other than the primary one. So for instance certificateStatus has a KEY on serial but not a UNIQUE one.

For certificateStatus we deal with that by using an UPDATE. That code also protects revokedCertificates from having duplicates. As we switch to revokedCertificates we'll want to make sure that revocations open a transaction, do a SELECT to make sure the row doesn't yet exist, and then INSERT.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, and I'll do that handling in #8408

@aarongable aarongable merged commit 9365990 into main Oct 2, 2025
13 checks passed
@aarongable aarongable deleted the revoked-serial-index branch October 2, 2025 23:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants