Skip to content

Conversation

@edwintorok
Copy link
Contributor

We got a failure in Jenkins:

ASSERT one_shot_success
--------------------------------------------------------------------------------
[failure] Error one_shot_success: expecting
true, got
false.

The test schedules a job for 1 second in the future and then checks that
it took (0.99, 2.01) s to run it.
But the scheduler, and indeed Thread.delay cannot make such guarantees.
It can only ensure that the job is scheduled at least 1 s in the future.
If the system is busy it can take 2s, 30s, 10m, etc. the kernel provides
no upper bound, this is not a real-time operating system.

Replace the strict check with a timeout based one instead: wait up to a
maximum allowed time, and then check how soon the callback actually got
invoked (i.e. that it didn't run too early).
We need the maximum allowed time because we don't want the test to be
stuck forever, but we also want the test to stop as soon as the callback
is executed, i.e. so it doesn't needlessly wait a minute on each test if
the scheduler actually finished in 1s.

Signed-off-by: Edwin Török [email protected]

@edwintorok edwintorok requested a review from lindig February 17, 2020 15:25
Copy link
Contributor

@lindig lindig left a comment

Choose a reason for hiding this comment

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

Less code and more reliable.

We got a failure in Jenkins:
```
ASSERT one_shot_success
--------------------------------------------------------------------------------
[failure] Error one_shot_success: expecting
true, got
false.
```

The test schedules a job for 1 second in the future and then checks that
it took `(0.99, 2.01) s` to run it.
But the scheduler, and indeed Thread.delay cannot make such guarantees.
It can only ensure that the job is scheduled *at least* 1 s in the future.
If the system is busy it can take 2s, 30s, 10m, etc. the kernel provides
no upper bound, this is not a real-time operating system.

Replace the strict check with a timeout based one instead: wait up to a
maximum allowed time, and then check how soon the callback actually got
invoked (i.e. that it didn't run too early).
We need the maximum allowed time because we don't want the test to be
stuck forever, but we also want the test to stop as soon as the callback
is executed, i.e. so it doesn't needlessly wait a minute on each test if
the scheduler actually finished in 1s.

Signed-off-by: Edwin Török <[email protected]>
@edwintorok edwintorok force-pushed the private/edvint/scheduler branch from 3eaa923 to 8b97c21 Compare February 20, 2020 11:43
@psafont psafont merged commit eec30b6 into xapi-project:master Feb 26, 2020
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.

4 participants