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

Ensure consistent cancellation of chained dependent jobs #4590

Merged
merged 2 commits into from Apr 4, 2022

Conversation

Martchus
Copy link
Contributor

@Martchus Martchus commented Apr 1, 2022

  • Cancel regularly and directly chained dependencies consistently only down
    the chain
    • Keep the behavior for parallel dependencies as-is (so they are still
      canceled up the hierarchy except PARALLEL_CANCEL_WHOLE_CLUSTER is set
      to a falsy value)
    • Cancellation of the entire chain at once is still possible by
      canceling from the root (unless there are multiple roots)
    • The cancellation up the chain was already prevented before in certain
      cases but whether it worked depended on the internal order in which
      dependencies have been traversed
    • Be more in-line with user expectations, at least judging by
      https://progress.opensuse.org/issues/108476 it expected that
      cancellation only affects jobs down the chain
  • Track whether the job cluster is computed for cancellation on all levels
    of nested functions calls to ensure a consistent behavior regardless of
    the internal order of dependency traversal

No tests yet because I hope failing unit tests will give me a hint were to make adjustments.

@codecov
Copy link

codecov bot commented Apr 1, 2022

Codecov Report

Merging #4590 (2c1880a) into master (bccf1cf) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #4590   +/-   ##
=======================================
  Coverage   97.98%   97.98%           
=======================================
  Files         375      375           
  Lines       34415    34419    +4     
=======================================
+ Hits        33723    33727    +4     
  Misses        692      692           
Impacted Files Coverage Δ
lib/OpenQA/Schema/Result/Jobs.pm 98.39% <100.00%> (+<0.01%) ⬆️
t/05-scheduler-cancel.t 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bccf1cf...2c1880a. Read the comment docs.

@Martchus Martchus marked this pull request as ready for review April 1, 2022 15:35
Copy link
Member

@okurz okurz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

everything ok maybe except the word "depended" which I am not so sure about. Shouldn't it be "dependant"?

@Martchus
Copy link
Contributor Author

Martchus commented Apr 1, 2022

According to the dictionary "dependent" is the most common form: https://www.oxfordlearnersdictionaries.com/definition/american_english/dependent_1

* Cancel regularly and directly chained dependencies consistently only down
  the chain
    * Keep the behavior for parallel dependencies as-is (so they are still
      canceled up the hierarchy except PARALLEL_CANCEL_WHOLE_CLUSTER is set
      to a falsy value)
    * Cancellation of the entire chain at once is still possible by
      canceling from the root (unless there are multiple roots)
    * The cancellation up the chain was already prevented before in certain
      cases but whether it worked dependent on the internal order in which
      dependencies have been traversed
    * Be more in-line with user expectations, at least judging by
      https://progress.opensuse.org/issues/108476 it expected that
      cancellation only affects jobs down the chain
* Track whether the job cluster is computed for cancellation on all levels
  of nested functions calls to ensure a consistent behavior regardless of
  the internal order of dependency traversal
@okurz okurz changed the title Ensure consistent cancellation of chained depended jobs Ensure consistent cancellation of chained dependent jobs Apr 4, 2022
@mergify mergify bot merged commit 194e7cd into os-autoinst:master Apr 4, 2022
@Martchus Martchus deleted the cancel branch April 4, 2022 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants