Add ability to cancel PKI tidy operations, pause between tidying certs #16958
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When tidy operations take a long time to execute (and especially when executing them automatically), having the ability to cancel them becomes useful to reduce strain on Vault clusters (and let them be rescheduled at a later time). To this end, we add the /tidy-cancel write endpoint. Signed-off-by: Alexander Scheel <[email protected]>
Signed-off-by: Alexander Scheel <[email protected]>
By setting pause_duration, operators can have a little control over the resource utilization of a tidy operation. While the list of certificates remain in memory throughout the entire operation, a pause is added between processing certificates and the revocation lock is released. This allows other operations to occur during this gap and potentially allows the tidy operation to consume less resources per unit of time (due to the sleep -- though obviously consumes the same resources over the time of the operation). Signed-off-by: Alexander Scheel <[email protected]>
Signed-off-by: Alexander Scheel <[email protected]>
Signed-off-by: Alexander Scheel <[email protected]>
Signed-off-by: Alexander Scheel <[email protected]>
cipherboy
force-pushed
the
cipherboy-add-tidy-cancel
branch
from
f243557 to
f9ba5d7
Compare
Signed-off-by: Alexander Scheel <[email protected]>
| @@ -164,6 +194,11 @@ func (b *backend) startTidyOperation(req *logical.Request, config *tidyConfig) { | |||
| } | |||
| } | |||
|
|
|||
| // Check for cancel before continuing. | |||
There was a problem hiding this comment.
Should we also schedule if case a cancel comes in after the doTidy func but before we release tidyCASGuard? -> defer atomic.StoreUint32(b.tidyCancelCAS, 0)
There was a problem hiding this comment.
If anything, I think we just need a atomic.CompareAndSwapUint32(b.tidyCancelCAS, 1, 0) at the top of this function before getting the tidyCASGuard. otherwise, I don't think it really matters, IMO?
There was a problem hiding this comment.
Sure that would work as well.
There was a problem hiding this comment.
(I think that's correct because it ensures that if a cancel somehow came in after we finished, we'd want any new ones to start fresh).
Signed-off-by: Alexander Scheel <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds two features to the PKI tidy operation that become useful with auto-tidy: