Skip to content

Simplify & speed up data_destroy timing tests #3463

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Feb 26, 2018
Merged

Simplify & speed up data_destroy timing tests #3463

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
231 changes: 92 additions & 139 deletions ocaml/xapi/test_vdi_cbt.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -348,163 +348,116 @@ let test_data_destroy =
(Db.VDI.get_type ~__context ~self:vdi)
in

(** If [realistic_timing] is true, the tests will use the Message_forwarding module and
the original timeouts, otherwise they will call the implementation in
Xapi_vdi directly with a smaller timeout to make the tests faster. The
former leads to more comprehensive tests, as it goes through message
forwarding, which also includes a timeout / retry mechanism, and can thus
impact the timing of data_destroy: when the VBD unplug is already in
progress, Xapi_vdi.check_operation_error and the message forwarding layer
will first wait for that operation to finish. *)
let test_data_destroy_timing ~realistic_timing =
let speedup = if realistic_timing then 1.0 else 10.0 in
(* VDI.data_destroy currently waits for at most 4 seconds for the VBDs of
the VDI to be unplugged and destroyed *)
let timeout = 4.0 /. speedup in
(* We simulate a VBD.unplug call, which might take 1-2 seconds *)
let vbd_unplug_time = 1.5 /. speedup in
let () = assert (vbd_unplug_time < timeout) in
let other_delay = 0.2 /. speedup in
(* We have to leave some extra time for data_destroy to finish, otherwise
the tests will fail with a stricter timeout. *)
let timebox_timeout_delay = 1.0 in

let uses_client = realistic_timing in
let call_data_destroy ~__context ~self =
if uses_client then begin
Api_server.Forwarder.VDI.data_destroy ~__context ~self
end else
Xapi_vdi._data_destroy ~__context ~self ~timeout
in

let start_vbd_unplug ~__context self =
Db.VBD.set_current_operations ~__context ~self ~value:["x", `unplug]
in
let finish_vbd_unplug ~__context self =
Db.VBD.set_currently_attached ~__context ~self ~value:false;
Db.VBD.set_current_operations ~__context ~self ~value:[];
in
let test_data_destroy_timing =
let bg f = Thread.create f () in

(* Creates a VBD that is currently_attached to our VDI *)
let setup_test () =
let __context, sR, vDI = setup_test_for_data_destroy () in
let vM = Test_common.make_vm ~__context () in
let vbd = Test_common.make_vbd ~__context ~vDI ~vM ~currently_attached:true () in
(__context, vDI, vbd)
let start_vbd_unplug () =
Db.VBD.set_current_operations ~__context ~self:vbd ~value:["x", `unplug]
in
let finish_vbd_unplug () =
Db.VBD.set_currently_attached ~__context ~self:vbd ~value:false;
Db.VBD.set_current_operations ~__context ~self:vbd ~value:[]
in
let destroy_vbd () = Db.VBD.destroy ~__context ~self:vbd in
let data_destroy ~timeout =
(* We need a 1-second tolerance here, otherwise the test will fail *)
let timebox_timeout = timeout +. 1.0 in
Helpers.timebox
~timeout:timebox_timeout
~otherwise:(fun () -> Alcotest.fail (Printf.sprintf "data_destroy did not return in %f seconds" timebox_timeout))
(fun () -> Xapi_vdi._data_destroy ~__context ~self:vDI ~timeout)
in
(vDI, start_vbd_unplug, finish_vbd_unplug, destroy_vbd, data_destroy)
in

let test_data_destroy_succeeds =
(* The parameters tell us whether the VBD unplug operation has started/finished by the time we call VDI.data_destroy. *)
let test_data_destroy_succeeds ~vbd_unplug_started ~vbd_unplug_finished () =
if vbd_unplug_finished && (not vbd_unplug_started) then failwith "VBD.unplug finished but not started by the time VDI.data_destroy is called: impossible";

let __context, vDI, vbd = setup_test () in
if vbd_unplug_started then start_vbd_unplug ~__context vbd;
if vbd_unplug_finished then finish_vbd_unplug ~__context vbd;

let _: Thread.t = Thread.create (fun () ->
if not vbd_unplug_started then begin
Thread.delay other_delay;
start_vbd_unplug ~__context vbd
end;
if not vbd_unplug_finished then begin
Thread.delay vbd_unplug_time;
finish_vbd_unplug ~__context vbd
end;
Thread.delay other_delay;
Db.VBD.destroy ~__context ~self:vbd
) ()
in
let timeout = timeout +. timebox_timeout_delay in
Helpers.timebox
~timeout
~otherwise:(fun () -> failwith "data_destroy did not finish successfully in 5 seconds")
(fun () -> call_data_destroy ~__context ~self:vDI)
in
[ "test_data_destroy_succeeds_when_vbd_is_already_unplugged", `Slow, test_data_destroy_succeeds ~vbd_unplug_started:true ~vbd_unplug_finished:true
; "test_data_destroy_succeeds_when_vbd_is_being_unplugged", `Slow, test_data_destroy_succeeds ~vbd_unplug_started:true ~vbd_unplug_finished:false
; "test_data_destroy_succeeds_when_vbd_unplug_is_started_later", `Slow, test_data_destroy_succeeds ~vbd_unplug_started:false ~vbd_unplug_finished:false
]
let test_data_destroy_succeeds_when_unplug_starts_later () =
let _vdi, start_vbd_unplug, finish_vbd_unplug, destroy_vbd, data_destroy = setup_test () in
let t = bg (fun () ->
Thread.delay 0.05;
start_vbd_unplug ();
Thread.delay 0.05;
finish_vbd_unplug ();
Thread.delay 0.05;
destroy_vbd ()
) in
data_destroy ~timeout:0.4;
Thread.join t
in

let test_data_destroy_times_out =
(* The parameters tell us whether the VBD unplug operation has started/finished by the time VDI.data_destroy times out. *)
let test_data_destroy_times_out ~vbd_unplug_started ~vbd_unplug_finished () =
let __context, vDI, vbd = setup_test () in

let t = Thread.create (fun () ->
(* The data_destroy operation will time out in 4 seconds *)
let unplug_start_delay = match vbd_unplug_started, vbd_unplug_finished with
| true, true -> timeout -. vbd_unplug_time -. other_delay (* unplug starts & finishes before the timeout *)
| true, false -> timeout -. (vbd_unplug_time /. 2.0) (* unplug starts before & finishes after the timeout *)
| false, false -> timeout +. other_delay (* unplug starts after the 4s timeout *)
| _ -> failwith "VBD.unplug finished but not started by the time VDI.data_destroy times out: impossible"
in
assert (unplug_start_delay >= 0.0);
Thread.delay unplug_start_delay;
start_vbd_unplug ~__context vbd;
(* Simulate a VBD.unplug call, which might take 1-2 seconds *)
Thread.delay vbd_unplug_time;
(* We have to finish the VBD unplug in the unit test, otherwise
message forwarding will wait for a long time for the in-progress
VBD.unplug operation to finish *)
finish_vbd_unplug ~__context vbd
) ()
in
(* If the timebox expires, we will fail because we will get an unexpected exception *)
let timeout = timeout +. timebox_timeout_delay in
Alcotest.check_raises "data_destroy times out"
Api_errors.(Server_error (vdi_in_use, [Ref.string_of vDI; "data_destroy"]))
(fun () ->
Helpers.timebox
~timeout
~otherwise:(fun () -> failwith "data_destroy did not time out")
(fun () -> call_data_destroy ~__context ~self:vDI)
);
(* Wait for the background thread to finish to prevent it failing with
DBCache_NotFound exceptions due to missing references - this
probably happens when it tries to use the VBD reference but its __context already
got garbage collected *)
Thread.join t
in
let test_data_destroy_succeeds_when_vbd_is_being_unplugged () =
let _vdi, start_vbd_unplug, finish_vbd_unplug, destroy_vbd, data_destroy = setup_test () in
let t = bg (fun () ->
start_vbd_unplug ();
Thread.delay 0.05;
finish_vbd_unplug ();
Thread.delay 0.05;
destroy_vbd ()
) in
Thread.delay 0.02;
data_destroy ~timeout:0.4;
Thread.join t
in

let test_data_destroy_times_out_when_nothing_happens_to_vbd () =
let __context, vDI, vbd = setup_test () in
let timeout = timeout +. timebox_timeout_delay in
(* If the timebox expires, we will fail because we will get an unexpected exception *)
Alcotest.check_raises "data_destroy times out"
Api_errors.(Server_error (vdi_in_use, [Ref.string_of vDI; "data_destroy"]))
(fun () ->
Helpers.timebox
~timeout
~otherwise:(fun () -> failwith (Printf.sprintf "data_destroy did not time out in %f seconds" timeout))
(fun () -> call_data_destroy ~__context ~self:vDI)
)
in
[ "test_data_destroy_times_out_when_vbd_does_not_get_destroyed_in_time", `Slow, test_data_destroy_times_out ~vbd_unplug_started:true ~vbd_unplug_finished:true
; "test_data_destroy_times_out_when_vbd_unplug_does_not_finish_in_time", `Slow, test_data_destroy_times_out ~vbd_unplug_started:true ~vbd_unplug_finished:false
; "test_data_destroy_times_out_when_vbd_unplug_is_not_started_in_time", `Slow, test_data_destroy_times_out ~vbd_unplug_started:false ~vbd_unplug_finished:false
; "test_data_destroy_times_out_when_nothing_happens_to_vbd", `Slow, test_data_destroy_times_out_when_nothing_happens_to_vbd
]
let test_data_destroy_succeeds_when_vbd_is_being_destroyed () =
let _vdi, start_vbd_unplug, finish_vbd_unplug, destroy_vbd, data_destroy = setup_test () in
let t = bg (fun () ->
start_vbd_unplug ();
finish_vbd_unplug ();
Thread.delay 0.05;
destroy_vbd ()
) in
Thread.delay 0.02;
data_destroy ~timeout:0.4;
Thread.join t
in

let suffix = if realistic_timing then "_with_realistic_timing" else "_with_simulated_timing" in
(test_data_destroy_succeeds
@ test_data_destroy_times_out)
|> List.map (function (n, s, t) -> (n ^ suffix, s, t))
in
let test_data_destroy_times_out_when_vbd_does_not_get_unplugged_in_time () =
let vDI, start_vbd_unplug, finish_vbd_unplug, destroy_vbd, data_destroy = setup_test () in
let t = bg (fun () ->
Thread.delay 0.05;
start_vbd_unplug ();
(* finish_vbd_unplug does not happen in time *)
) in
Alcotest.check_raises "data_destroy should raise VDI_IN_USE after its timeout"
Api_errors.(Server_error (vdi_in_use, [Ref.string_of vDI; "data_destroy"]))
(fun () -> data_destroy ~timeout:0.4);
Thread.join t
in

let test_data_destroy_times_out_when_vbd_does_not_get_destroyed_in_time () =
let vDI, start_vbd_unplug, finish_vbd_unplug, destroy_vbd, data_destroy = setup_test () in
let t = bg (fun () ->
Thread.delay 0.05;
start_vbd_unplug ();
Thread.delay 0.05;
finish_vbd_unplug ();
(* destroy_vbd does not happen in time *)
) in
Alcotest.check_raises "data_destroy should raise VDI_IN_USE after its timeout"
Api_errors.(Server_error (vdi_in_use, [Ref.string_of vDI; "data_destroy"]))
(fun () -> data_destroy ~timeout:0.4);
Thread.join t
in

(** This flag controls whether we should run the {!test_data_destroy_timing}
tests with realistic timing (with [realistic_timing = true). *)
let run_slow_tests = true in
let slow_tests = test_data_destroy_timing ~realistic_timing:true in
[ "test_data_destroy_succeeds_when_unplug_starts_later", `Slow, test_data_destroy_succeeds_when_unplug_starts_later
; "test_data_destroy_succeeds_when_vbd_is_being_unplugged", `Slow, test_data_destroy_succeeds_when_vbd_is_being_unplugged
; "test_data_destroy_succeeds_when_vbd_is_being_destroyed", `Slow, test_data_destroy_succeeds_when_vbd_is_being_destroyed
; "test_data_destroy_times_out_when_vbd_does_not_get_unplugged_in_time", `Slow, test_data_destroy_times_out_when_vbd_does_not_get_unplugged_in_time
; "test_data_destroy_times_out_when_vbd_does_not_get_destroyed_in_time", `Slow, test_data_destroy_times_out_when_vbd_does_not_get_destroyed_in_time
]

in

[ "test_vdi_after_data_destroy", `Quick, test_vdi_after_data_destroy
; "test_vdi_managed_data_destroy", `Quick, test_vdi_managed_data_destroy
; "test_data_destroy_does_not_change_vdi_type_to_cbt_metadata_if_it_fails", `Quick, test_does_not_change_vdi_type_to_cbt_metadata_if_it_fails
] @ test_data_destroy_timing ~realistic_timing:false
@ (if run_slow_tests then slow_tests else [])
] @ test_data_destroy_timing

(* behaviour verification VDI.list_changed_blocks *)
let test_vdi_list_changed_blocks () =
let __context = Test_common.make_test_database () in
let sR = make_mock_server_infrastructure ~__context in
Expand Down