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

scheduler: Handle cycles in directly chained dependencies #3490

Merged

Conversation

Martchus
Copy link
Contributor

Otherwise the unhandled exception prevents the scheduler from processing
any further scheduled jobs and jobs are stuck in the scheduled state.

See https://progress.opensuse.org/issues/32545#note-37 for a graph with
an example of such a problematic directly chained cluster.

Otherwise the unhandled exception prevents the scheduler from processing
*any* further scheduled jobs and jobs are stuck in the scheduled state.

See https://progress.opensuse.org/issues/32545#note-37 for a graph with
an example of such a problematic directly chained cluster.
@codecov
Copy link

codecov bot commented Oct 27, 2020

Codecov Report

Merging #3490 into master will increase coverage by 0.05%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3490      +/-   ##
==========================================
+ Coverage   95.52%   95.57%   +0.05%     
==========================================
  Files         366      366              
  Lines       31632    31660      +28     
==========================================
+ Hits        30215    30259      +44     
+ Misses       1417     1401      -16     
Impacted Files Coverage Δ
lib/OpenQA/Scheduler/Model/Jobs.pm 93.67% <100.00%> (+0.15%) ⬆️
t/05-scheduler-dependencies.t 99.32% <100.00%> (+0.02%) ⬆️
t/05-scheduler-full.t 97.94% <0.00%> (+0.51%) ⬆️
t/lib/OpenQA/SeleniumTest.pm 89.07% <0.00%> (+1.09%) ⬆️
t/lib/OpenQA/Test/Utils.pm 68.94% <0.00%> (+3.70%) ⬆️

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 14ea04f...cea9373. Read the comment docs.

so they prevent other jobs from being assigned
Copy link
Member

@kalikiana kalikiana left a comment

Choose a reason for hiding this comment

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

Note: Unrelated failures due to poo#75370 but relevant tests are passing.

Looks sensible to me. And thank you for linking the semi-related ticket!

@@ -141,6 +143,33 @@ subtest 'assign multiple jobs to worker' => sub {
ok($job_token, 'job token present');
};

# prevent writing to a log file to enable use of combined_like in the following tests
my $usual_log = $t->app->log;
$t->app->log(Mojo::Log->new(level => 'debug'));
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this should just be enabled at the start of the test? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, it shouldn't. I also intentionally disable it immediately after the tests using it again. That's because other logs messages would otherwise interfere with the test output (unless explicitly handled).

Copy link
Member

Choose a reason for hiding this comment

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

can you do that within the subtest as well as the restoring?

@Martchus
Copy link
Contributor Author

Martchus commented Oct 27, 2020

Ah, nice that you've already created a ticket for the failure. Unfortunately I'm currently not sure where it comes from. And the OBS failures should hopefully go away with #3491.

@Martchus Martchus merged commit 12691a1 into os-autoinst:master Oct 28, 2020
@Martchus Martchus deleted the fix-scheduler-directly-chained-dependency branch October 28, 2020 08:46
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