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

Faster async metadata signing #682

Closed
sdherr opened this issue Nov 23, 2022 · 1 comment · Fixed by #683
Closed

Faster async metadata signing #682

sdherr opened this issue Nov 23, 2022 · 1 comment · Fixed by #683
Labels
.feature CHANGES/<issue_number>.feature Triage-Needed

Comments

@sdherr
Copy link
Contributor

sdherr commented Nov 23, 2022

Metadata signing is currently iterative and synchronous, and therefore potentially very slow. The publish task currently iterates through the Releases in a repo in a for loop, generating, signing, and saving the metadata for them one-at-a-time. Some people are in environments where signing is not simply a call out to local gpg, but rather a call to some other service owned by some other team. If a single signing request takes 3 minutes on average, and you have a repository with 15 releases, then waiting for the signing of the repodata alone takes 45 minutes. And in the mean time it blocks an entire Pulp worker that is completely idle, and blocks all other changes to the repo.

It is possible, and in fact fairly easy, to divide that task up differently so that we generate all the metadata, concurrently request all the signatures, and then save all the results. That would speed things up considerably, finishing in 3 or 4 minutes regardless of how large the repo is. O(1) vs O(n), assuming that the limiting factor is the signing. PR incoming.

@sdherr
Copy link
Contributor Author

sdherr commented Nov 23, 2022

Pulpcore SigningService even already has an asynchronous asign method to complement the synchronous sign method, we just have to use it.

sdherr added a commit to sdherr/pulp_deb that referenced this issue Nov 23, 2022
…eeding up the publish task in environments where signing is slow.

closes pulp#682
sdherr added a commit to sdherr/pulp_deb that referenced this issue Nov 23, 2022
…eeding up the publish task in environments where signing is slow.

closes pulp#682
sdherr added a commit to sdherr/pulp_deb that referenced this issue Nov 23, 2022
…eeding up the publish task in environments where signing is slow.

closes pulp#682
sdherr added a commit to sdherr/pulp_deb that referenced this issue Dec 8, 2022
Sign all apt releases in a repo concurrently.

This PR splits the previous `_ReleaseHelper.finish()` function into three parts, `save_unsigned_metadata`, `sign_metadata`, and `save_signed_metadata`, and then calls them in order, doing the signing concurrently.

I've tested in PPE now by syncing the azure-cli repo to PPE. Before this change running a publish task took **45 minutes**. After this change was applied signing takes about **4 minutes**. A 10x speedup is in line with expectations, given that there are 16 distros in that repo and not _everything_ is just sitting around waiting for signing. I've increase the polling to the signer container from once a minute to every 10 seconds to help with this.

There was a small setback the first time I tried it where the process hit the cpu/memory constraints and died. Those have been increase appropriately to ensure a good minimum for concurrently signing big apt repos, and there's actually no limit any more so even if it goes above that it won't die unless the node runs out of space.

Bug/PR filed upstream: pulp#682

Related work items: #16101470
sdherr added a commit to sdherr/pulp_deb that referenced this issue Jan 31, 2023
Sign all apt releases in a repo concurrently.

This PR splits the previous `_ReleaseHelper.finish()` function into three parts, `save_unsigned_metadata`, `sign_metadata`, and `save_signed_metadata`, and then calls them in order, doing the signing concurrently.

I've tested in PPE now by syncing the azure-cli repo to PPE. Before this change running a publish task took **45 minutes**. After this change was applied signing takes about **4 minutes**. A 10x speedup is in line with expectations, given that there are 16 distros in that repo and not _everything_ is just sitting around waiting for signing. I've increase the polling to the signer container from once a minute to every 10 seconds to help with this.

There was a small setback the first time I tried it where the process hit the cpu/memory constraints and died. Those have been increase appropriately to ensure a good minimum for concurrently signing big apt repos, and there's actually no limit any more so even if it goes above that it won't die unless the node runs out of space.

Bug/PR filed upstream: pulp#682

Related work items: #16101470
@hstct hstct closed this as completed in ffea67c Mar 8, 2023
hstct added a commit that referenced this issue Mar 8, 2023
#682: Sign the metadata for all releases in a repo concurrently, greatly speeding up the publish task in environments where signing is slow.
adamsanaglo pushed a commit to adamsanaglo/pulpcore that referenced this issue Jul 13, 2023
Sign all apt releases in a repo concurrently.

This PR splits the previous `_ReleaseHelper.finish()` function into three parts, `save_unsigned_metadata`, `sign_metadata`, and `save_signed_metadata`, and then calls them in order, doing the signing concurrently.

I've tested in PPE now by syncing the azure-cli repo to PPE. Before this change running a publish task took **45 minutes**. After this change was applied signing takes about **4 minutes**. A 10x speedup is in line with expectations, given that there are 16 distros in that repo and not _everything_ is just sitting around waiting for signing. I've increase the polling to the signer container from once a minute to every 10 seconds to help with this.

There was a small setback the first time I tried it where the process hit the cpu/memory constraints and died. Those have been increase appropriately to ensure a good minimum for concurrently signing big apt repos, and there's actually no limit any more so even if it goes above that it won't die unless the node runs out of space.

Bug/PR filed upstream: pulp/pulp_deb#682

Related work items: #16101470
@quba42 quba42 added .feature CHANGES/<issue_number>.feature and removed Feature labels Aug 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
.feature CHANGES/<issue_number>.feature Triage-Needed
Projects
Archived in project
2 participants