Skip to content

Commit

Permalink
CP-51690: [bugfix] Xapi_periodic_scheduler: avoid 10s sleep on empty …
Browse files Browse the repository at this point in the history
…queue

Wake up the scheduler immediately when there are more tasks.
Otherwise timeouts <10s may not work correctly, and it is difficult to test the
periodic scheduler if you need to wait 10s for it to start working.

If there are no tasks, then it will still sleep efficiently, but as soon as
more tasks are added (with [~signal:true], which is the default) it will
immediately wake up and calculate the next sleep time.

In practice it is probably quite rare for XAPI's queue to be empty (there are usually periodic tasks),
but we cannot rely on this.

Signed-off-by: Edwin Török <[email protected]>
  • Loading branch information
edwintorok committed Nov 18, 2024
1 parent 95dbc42 commit 68af6ce
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 1 deletion.
32 changes: 32 additions & 0 deletions ocaml/tests/test_event.ml
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,37 @@ let object_level_event_test _session_id =
if !failure then
Alcotest.fail "failed to see object-level event change"

let test_short_oneshot () =
(* don't call event_setup_common here, it'll register a dummy event and hide the bug *)
let started = ref false in
let m = Mutex.create () in
let cond = Condition.create () in
let scheduler () =
Mutex.lock m ;
started := true ;
Condition.broadcast cond ;
Mutex.unlock m ;
Xapi_periodic_scheduler.loop ()
in
ignore (Thread.create scheduler ()) ;
(* ensure scheduler sees an empty queue , by waiting for it to start *)
Mutex.lock m ;
while not !started do
Condition.wait cond m
done ;
Mutex.unlock m ;
(* run the scheduler, let it realize its queue is empty,
a Thread.yield is not enough due to the use of debug which would yield back almost immediately.
*)
Thread.delay 1. ;
let fired = Atomic.make false in
let fire () = Atomic.set fired true in
let task = "test_oneshot" in
Xapi_periodic_scheduler.add_to_queue task Xapi_periodic_scheduler.OneShot 1.
fire ;
Thread.delay 2. ;
assert (Atomic.get fired)

let test =
[
("test_event_from_timeout", `Slow, test_event_from_timeout)
Expand All @@ -287,4 +318,5 @@ let test =
; ("test_event_from", `Quick, event_from_test)
; ("test_event_from_parallel", `Slow, event_from_parallel_test)
; ("test_event_object_level_event", `Slow, object_level_event_test)
; ("test_short_oneshot", `Slow, test_short_oneshot)
]
2 changes: 1 addition & 1 deletion ocaml/xapi/xapi_periodic_scheduler.ml
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ let loop () =
while true do
let empty = with_lock lock (fun () -> Ipq.is_empty queue) in
if empty then
Thread.delay 10.0
wait_next 10.0
(* Doesn't happen often - the queue isn't usually empty *)
else
let next = with_lock lock (fun () -> Ipq.maximum queue) in
Expand Down

0 comments on commit 68af6ce

Please sign in to comment.