QA (public) » openQA Project (public)

Understand why #125276 could not fix the problem

The "service queue already full" problem was only "fixed" by #125276 in the sense that none of such jobs need to be restarted manually anymore. It does not look like this was necessary today:

openqa=# select id, clone_id from jobs where (id > 3244452) and (reason like '%service queue already full%' ) and (select user_id from audit_events where event_data like concat('%', jobs.id, '%') and event = 'job_restart' limit 1) is not null order by id desc;
 id | clone_id 
----+----------
(0 rows)

So my changes for #125276 work in the sense that the "service queue already full" problem shouldn't be a problem of our users. There can still be jobs ending up as incompletes with that reason, though.

Make sure jobs really restart if the cache service queue is full

I don't think there's evidence showing the restart is broken in this case.

Ok - I've adjusted the title and AC1 of this ticket.

Ok, so that's what happened:

1. Job 1 (the current on in the attached screenshot, https://openqa.opensuse.org/tests/3243518) has been restarted automatically as the cache queue was full. The reason for the restart doesn't matter, though. If a user had clicked the restart button the behavior would have been the same.
   • So this first step is normal behavior and must be handled well.
2. The chained child of job 1 (job 2, https://openqa.opensuse.org/tests/3243519) is skipped.
   • That is definitely what should happen.
3. The parallel child of job 2 (job 3, https://openqa.opensuse.org/tests/3243701) is skipped as well.
   • That is still what should happen.
4. The chained parent of job 3 (job 4, the "other root" in the attached screenshot, https://openqa.opensuse.org/tests/3243402) is restarted. It ends up with the result parallel_restarted because a job being restarted as dependency normally only happens for parallel dependency. However, here it happened because a chained child has been skipped.
   • That supposedly should not have happened. I'll have to ponder more about it though (e.g. what further steps would have looked like if it would not have happened in different cases like job 4 needed to be restarted itself).
   • The code does not account for this situation. It assumes that a depended job that is running must be a parallel dependency. Any chained parent is assumed to have already been finished. Any chained child is assumed to have not been started yet. However, as we have seen, these assumptions are not correct.
   • The result parallel_restarted is not completely wrong, though. In fact, a related parallel job has been restarted. It is just that this dependency is only indirect.
   • If a job is restarted as dependency like job 4 it is aborted. This means no further logs/assets are uploaded. So that job 4 has no logs is actually not a bug.
5. The chained children of job 4 are not skipped.
   • This is because job 4 is concluded in a state where its result is already known (as parallel_restarted is assigned in 4.). The behavior in this state is an explicit choice and could easily be changed. I'll have to ponder what other problems would arise if we would change it, though.
   • This also means the job networks of those children are never cleaned up.
   • If we would change the behavior by removing defined $new_val{result} in the condition in sub done to run _job_stop_cluster also in this state then the job would without further tweaking end up as parallel_failed. That would supposedly be even more misleading than parallel_restarted.
6. Related jobs of job 4 are still considered by the scheduler due to 5.
   • This supposedly leads to the errors mentioned in #128267#note-5 because networks haven't been correctly released.
7. The chained children of job 4 are started at some point. So they run despite the asset that job 4 should have been publishing is missing which explains the 404 error of the cache service download.
   • So the cache service is not at fault here; the asset was really missing at this point.

That means the behavior should likely be changed so:

1. Only parallel jobs (that are running) are restarted as dependencies.
2. If a job ends up parallel_restarted its children should be skipped as well right away and not just when the worker reports the job is done. Otherwise, if the worker would not respond anymore for some reason we'd still accidentally execute those children.
3. Job networks should maybe be release when a job is aborted - or is that too early? Maybe the scheduler code should also be able to deal with the situation of already existing job networks better.

Setting due date based on mean cycle time of SUSE QE Tools

I haven't seen these log messages anymore in recent o3 and OSD logs. So the situation is eventually self-resolving. However, the code for allocating job networks is very wrong:

Using a transaction makes not really sense. It has been added by https://github.com/os-autoinst/openQA/commit/3c52abbe3d6364c02f73c6f9fe4afe44f89688f6 because find_or_create does not handle concurrent insertions. However, I don't see that we actually do any concurrent insertions. Let's assume I'm wrong and we do them. Then it still doesn't make sense because the error is not handled correctly. If the transaction would fail the code would simply attempt the find-insertion again. That would work but lead to the situation where $created is falsy. In this case the for-loop would continue forever at the return in the nested if-block is never reached. In case a fatal error occurs it would also loop forever because the next; doesn't work inside the catch block¹. So supposedly we never really run into these problematic code paths. I have simplified the code to avoid the transaction but kept an explicit error message in case the network already exists (so in case this really does happen we can easily distinguish this case and possibly tweak the behavior as needed).

When creating networks for the whole cluster a network may already exist. That's what the error messages from my previous comment are about. I guess it is ok to simply not treat this as an error. Likely the job is in a state where it doesn't need a different network allocated anymore anyways.

I've also added a test case to see what would generally happen if job networks are allocated again when they haven't been properly released before. This is generally handled well. So I guess we don't have to make releasing job networks more reliable.

PR with improvements for job networks: https://github.com/os-autoinst/openQA/pull/5111

¹

main::(-e:1):   1
  DB<1> use Try::Tiny;

  DB<2> try { die 'foo' } catch { print "caught: $_\n" ; next };
caught: foo at (eval 8)[/usr/lib/perl5/5.36.0/perl5db.pl:742] line 2.

  DB<3> for $i (1..4) { try { die "foo: $i" } catch { print "caught: $_\n" ; next }; }
caught: foo: 1 at (eval 9)[/usr/lib/perl5/5.36.0/perl5db.pl:742] line 2.
caught: foo: 2 at (eval 9)[/usr/lib/perl5/5.36.0/perl5db.pl:742] line 2.
caught: foo: 3 at (eval 9)[/usr/lib/perl5/5.36.0/perl5db.pl:742] line 2.
caught: foo: 4 at (eval 9)[/usr/lib/perl5/5.36.0/perl5db.pl:742] line 2.

All PRs have been merged. I think that's good enough for now. The restarting itself has been fixed and the handling of networks has been improved as well. I've checked jobs in production but there's no further example. I suppose testing this via unit tests must suffice. The same counts for the problem with allocating networks.