Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Smoother scheduling and leasing for crl-updater #7010

Merged
merged 9 commits into from
Sep 8, 2023
Merged

Conversation

aarongable
Copy link
Contributor

@aarongable aarongable commented Jul 22, 2023

Overhaul crl-updater's default (i.e. non-runOnce) behavior to update individual CRL shards continuously, rather than updating all shards in a large batch.

To accomplish this, it spins up one goroutine for each shard of each issuer this updater is responsible for. Each goroutine is solely responsible for its assigned shard. It sleeps for a random amount of time (to stagger their starts), then begins a ticker to wake up every updateInterval and re-issue its shard.

As part of this change, refactor updater.go into three separate files (batch.go, continuous.go, and updater.go) containing functions dedicated to single-run batch processing, long-running continuous processing, and shared helpers, respectively.

IN-9475 tracks the deprecation of the updateOffset config key. The other configuration changes in this PR do not require production changes.

Fixes #7023
DO NOT MERGE until after #7009
DO NOT MERGE until after #7051
DO NOT MERGE until after #7052

Nota bene: I strongly recommend reviewing this PR one commit at a time. The first few are just moving code around with no changes.

Base automatically changed from deprecate-leasing to main August 7, 2023 22:17
@aarongable aarongable changed the base branch from main to improve-update-crl-shard August 24, 2023 23:34
@github-actions
Copy link
Contributor

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

@aarongable aarongable marked this pull request as ready for review August 25, 2023 21:47
@aarongable aarongable requested a review from a team as a code owner August 25, 2023 21:47
@aarongable aarongable requested review from beautifulentropy and removed request for a team August 25, 2023 21:47
Copy link
Member

@beautifulentropy beautifulentropy left a comment

Choose a reason for hiding this comment

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

This all feels incredibly slick, great work!

@beautifulentropy beautifulentropy requested a review from a team August 29, 2023 20:49
Base automatically changed from improve-update-crl-shard to main August 31, 2023 16:06
@pgporada pgporada dismissed beautifulentropy’s stale review August 31, 2023 16:06

The base branch was changed.

@aarongable
Copy link
Contributor Author

The base branch was changed.

@pgporada FWIW, this doesn't require a re-review. This happens automatically when the first of two stacked changes is merged into main. This PR still represents the exact same set of commits and contains the exact same set of diffs -- it's just that now, instead of being based on top of some other PR branch, it is now based on top of main... which now contains the commits that were in that other PR branch!

@pgporada
Copy link
Member

pgporada commented Sep 1, 2023

I don't think I wrote that comment, it might have been a github automatic message? I for sure know I didn't open this PR yesterday.

Regardless, taking a look now.

@aarongable
Copy link
Contributor Author

I don't think I wrote that comment, it might have been a github automatic message?

Fascinating! I guess it was automatic, and attributed to you because you clicked the merge button on #7052. Apologies!

@aarongable aarongable merged commit 102b447 into main Sep 8, 2023
@aarongable aarongable deleted the crl-scheduling branch September 8, 2023 16:16
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.

crl-updater: update one shard at a time
3 participants