aptly icon indicating copy to clipboard operation
aptly copied to clipboard

Fix/publish concurrency

Open neolynx opened this issue 10 months ago • 2 comments

Replaces #1261

Description of the Change

Fixes #1125

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 avatar Apr 17 '24 18:04 neolynx

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.

neolynx avatar Apr 20 '24 21:04 neolynx

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

ramonnr avatar Apr 24 '24 12:04 ramonnr

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
@@            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.

codecov[bot] avatar Jun 08 '24 08:06 codecov[bot]

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 avatar Jun 08 '24 08:06 neolynx

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

Could you test and confirm ? Thanks !

neolynx avatar Jun 08 '24 10:06 neolynx