Fix/publish concurrency #1271
Open
Fix/publish concurrency #1271
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
neolynx
force-pushed
the
fix/publish-concurrency
branch
from
8464cad
to
346e782
Compare
|
the test t12_api:TaskAPITestParallelTasks failes with this implementation... does it need to be updated? Create Mirror in parallel seems now to return 202, but 409 was expected. |
neolynx
force-pushed
the
fix/publish-concurrency
branch
from
fa7b78b
to
37d521f
Compare
|
Sorry for the tardy response, I'll look over what happened after the rebase :) |
neolynx
force-pushed
the
fix/publish-concurrency
branch
from
37d521f
to
fb753df
Compare
This commit introduces a test which runs concurrent publishes (which could be parallell with multiproccessing, python is fun). The test purposly fails (at the point in history that this patch is written) in order to make it as easy as possible to verify later patches, which hopefully addresses concurrency problems. The same behaviour can easily be tested outside of the system tests with the following (or similar) shell $ aptly serve -listen=:8080 -no-lock $ aptly repo create create -distributions=testing local-repo $ atply publish repo -architectures=amd64 local-repo $ apt download aptly $ aptly repo add local-repo ./aptly*.deb $ for _ in $(seq 10); do curl -X PUT 127.0.0.1:8080/api/publish//testing In the local testing perfomed (on a dual core vm) the first 1-4 jobs would typically succeed and the rest would error out.
This commit blocks concurrent calls to RunTaskInBackground which is intended to fix the quirky behaviour where concurrent PUT calls to api/publish/<prefix>/<distribution> would immedietly reuturn an error. The solution proposed in this commit is not elegant and probaly has unintended side-effects. The intention of this commit is to highlight the area that actually needs to be addressed. Ideally this patch is amended or dropped entierly in favor of a better fixup.
neolynx
force-pushed
the
fix/publish-concurrency
branch
from
fb753df
to
348ea3e
Compare
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.
Replaces #1261
Description of the Change
Fixes #1125
This MR addresses a concurrency issue with the
api/publishendpoint, where concurrent PUTs typically fail. The MR in it self is not pretty, so consider this initial state of the MR a starting point of discussion.The commits are intentionally separated in order to make it as easy as possible to observe the failing test (and test it against other likely better code changes).