Allow configuring concurrent cleanup #4215
Conversation
Martchus
force-pushed
the
concurrent-cleanup
branch
from
0086cd2
to
ed85b94
Compare
Codecov Report
@@ Coverage Diff @@
## master #4215 +/- ##
=======================================
Coverage 97.91% 97.91%
=======================================
Files 369 369
Lines 33366 33387 +21
=======================================
+ Hits 32671 32692 +21
Misses 695 695
Continue to review full report at Codecov.
|
* Default behavior remains unchanged (all cleanup tasks block each other) * For simplicity this setting affects all cleanup tasks (and not just asset and result cleanup). * See https://progress.opensuse.org/issues/98922
* Improve phrasing of introduction * Add advanced cleanup section * Improve references * Fix positioning of element IDs (so one can see the heading when using internal links)
Martchus
force-pushed
the
concurrent-cleanup
branch
from
ed85b94
to
93ac34c
Compare
| # Specifies whether different cleanup tasks can run in parallel. This option | ||
| # makes particularly sense if assets and results are stored on different storage | ||
| # locations or if your common storage solution is performant enough. | ||
| concurrent = 0 |
There was a problem hiding this comment.
For some reason I thought this might take a number of tasks to run in parallel. Looking at the implementation clarified that of course... might be clear enough anyway, just wanted to mention it
There was a problem hiding this comment.
I hope the word "whether" in the comment makes it clear that this is just a flag.
There was a problem hiding this comment.
would it be possible to use bools like "true" or "false" here? This would clarify all ambiguity. If not I think mentioning it in the comment would make sense
There was a problem hiding this comment.
In Perl there's no true/false do this would require extra handling and would be inconsistent with other settings.
commit e6e0580 Merge: c0a92ba 93ac34c Author: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> AuthorDate: Wed Sep 22 15:38:49 2021 +0000 Commit: GitHub <noreply@github.com> CommitDate: Wed Sep 22 15:38:49 2021 +0000 Merge pull request os-autoinst#4215 from Martchus/concurrent-cleanup Allow configuring concurrent cleanup
| # prevent multiple limit_* tasks to run in parallel | ||
| return $job->retry({delay => 60}) | ||
| unless my $limit_guard = $app->minion->guard('limit_tasks', ONE_DAY); | ||
| return undef unless my $limit_guard = acquire_limit_lock_or_retry($job); |
There was a problem hiding this comment.
It looks like the _limit subroutine has duplicate logic at multiple places. Would it make sense to define a common subroutine for all to which you just pass the argument for guard()?
There was a problem hiding this comment.
Yes, it could likely be de-duplicated further.
and result cleanup).