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

Avoid running all suites at the same time at scheduling start #449

Merged
merged 1 commit into from
Dec 11, 2023

Conversation

jherbel
Copy link
Contributor

@jherbel jherbel commented Dec 7, 2023

This could lead to a high system load. Instead, we start as soon as the current timestamp is fully divisible by the execution interval.

@jherbel jherbel force-pushed the dev/joerg/scheduling branch 2 times, most recently from bf45ccf to 2a57121 Compare December 7, 2023 17:38
@jherbel jherbel changed the title Fix two minor scheduling issues Avoid running all suites at the same time at scheduling start Dec 7, 2023
@jherbel jherbel requested a review from SoloJacobs December 7, 2023 17:49
This could lead to a high system load. Instead, we start as soon as the
current timestamp is fully divisible by the execution interval.
@jherbel jherbel force-pushed the dev/joerg/scheduling branch from 2a57121 to 8992b00 Compare December 8, 2023 12:02
Copy link
Contributor

@SoloJacobs SoloJacobs left a comment

Choose a reason for hiding this comment

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

Two general remarks about this change:

  • It seems to me that calling tick().await on clock has a very similar effect. (at least if Interval::now() is relatively similar in all suites when the clocks are started.
  • I think the change itself needs to clarify whether this is addressing a theoretical problem or whether there is something concrete, which we observed in practice.

@@ -47,7 +58,7 @@ async fn run_suite_scheduler(suite: Suite) {
}

async fn run_cleanup_job(cancellation_token: CancellationToken, suites: Vec<Suite>) {
let mut clock = interval(Duration::from_secs(300));
let mut clock = interval_at(compute_start_time(300), Duration::from_secs(300));
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems to me that calling tick().await on clock has a very similiar effect.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure what you mean. If we dont use interval_at but interval, the first tick will complete immediately.

@jherbel
Copy link
Contributor Author

jherbel commented Dec 8, 2023

@SoloJacobs I discussed this with Simon, this a real problem (though not a severe one). See for example here:
https://github.com/elabit/robotmk/actions/runs/7141059520/job/19447631131#step:5:345
Before, with clokwerk, this was not the case:
https://github.com/elabit/robotmk/actions/runs/7100175288/job/19325716490#step:5:346

@jherbel jherbel requested a review from SoloJacobs December 8, 2023 12:28
@jherbel jherbel merged commit 4d88502 into main Dec 11, 2023
21 checks passed
@jherbel jherbel deleted the dev/joerg/scheduling branch December 11, 2023 08:53
@github-actions github-actions bot locked and limited conversation to collaborators Dec 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants