aptly-dev / aptly

aptly - Debian repository management tool
https://www.aptly.info/
MIT License
2.54k stars 369 forks source link

Fix/publish concurrency #1271

Closed neolynx closed 2 weeks ago

neolynx commented 2 months ago

Replaces #1261 Fixes #1276, #1125

Description of the Change

PR Updated: synchronous and asynchronous requests are queued until the resoures become available.

This MR addresses a concurrency issue with the api/publish endpoint, 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).

neolynx commented 2 months ago

the test t12_api:TaskAPITestParallelTasks failes with this implementation... does it need to be updated?

2024-04-20T21:03:23.8650330Z [GIN] 2024/04/20 - 21:03:23 | 202 | 12.838215884s |       127.0.0.1 | PUT      "/api/mirrors/x41qcXfiIf7ho72?_async=True"
2024-04-20T21:03:23.8662294Z Traceback (most recent call last):
2024-04-20T21:03:23.8675218Z   File "/home/runner/work/aptly/aptly/system/run.py", line 102, in run
2024-04-20T21:03:23.8676107Z     t.test()
2024-04-20T21:03:23.8676889Z   File "/home/runner/work/aptly/aptly/system/lib.py", line 178, in test
2024-04-20T21:03:23.8677717Z     self.check()
2024-04-20T21:03:23.8678774Z   File "/home/runner/work/aptly/aptly/system/t12_api/tasks.py", line 81, in check
2024-04-20T21:03:23.8680027Z     mirror_task_id, mirror_name = self._create_mirror(mirror_dist)
2024-04-20T21:03:23.8680826Z                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
2024-04-20T21:03:23.8681926Z   File "/home/runner/work/aptly/aptly/system/t12_api/tasks.py", line 25, in _create_mirror
2024-04-20T21:03:23.8683079Z     self.check_equal(resp2.status_code, 409)
2024-04-20T21:03:23.8684153Z   File "/home/runner/work/aptly/aptly/system/lib.py", line 418, in check_equal
2024-04-20T21:03:23.8684851Z     self.verify_match(a, b, match_prepare=pprint.pformat)
2024-04-20T21:03:23.8685526Z   File "/home/runner/work/aptly/aptly/system/lib.py", line 469, in verify_match
2024-04-20T21:03:23.8686425Z     raise Exception("content doesn't match:\n" + diff + "\n")
2024-04-20T21:03:23.8686894Z Exception: content doesn't match:
2024-04-20T21:03:23.8687342Z --- 
2024-04-20T21:03:23.8687629Z +++ 
2024-04-20T21:03:23.8688024Z @@ -1 +1 @@
2024-04-20T21:03:23.8688566Z -202
2024-04-20T21:03:23.8688908Z +409

Create Mirror in parallel seems now to return 202, but 409 was expected.

ramonnr commented 2 months ago

Sorry for the tardy response, I'll look over what happened after the rebase :)

codecov[bot] commented 3 weeks ago

Codecov Report

Attention: Patch coverage is 91.66667% with 7 lines in your changes missing coverage. Please review.

Project coverage is 74.63%. Comparing base (787f954) to head (fdce655).

Files Patch % Lines
api/api.go 55.55% 3 Missing and 1 partial :warning:
task/list.go 95.23% 2 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1271 +/- ## ========================================== - Coverage 74.86% 74.63% -0.24% ========================================== Files 144 144 Lines 16261 16299 +38 ========================================== - Hits 12174 12164 -10 - Misses 3149 3188 +39 - Partials 938 947 +9 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

neolynx commented 3 weeks ago

Hi !

I implemented a queue, async tasks are now executed squentially in order, as soon as the required resources are avaiilable.

could you give this a try ?

neolynx commented 3 weeks ago

Aptly is now also using the queue for synchronous tasks. this should fix the concurrency problem completely.

Could you test and confirm ? Thanks !