Skip to content

Commit

Permalink
CA-395512: process SMAPIv3 API calls concurrently (default off)
Browse files Browse the repository at this point in the history
By default message-switch calls are serialized for backwards compatibility reasons in the Lwt and Async modes.
(We tried enabling parallel actions by default but got some hard to debug failures in the CI).

This causes very long VM start times when multiple VBDs are plugged/unplugged concurrently: the operations are seen concurrently by message-switch,
but xapi-storage-script only retrieves and dispatches them sequentially, so any opportunity for parallel execution is lost.
Even though the actions themselves only take seconds, due to serialization, a VM start may take minutes.

Enable parallel processing explicitly here (instead of for all message-switch clients).
SMAPIv3 should expect to be called concurrently (on different hosts even), so in theory this change should be safe
and improve performance, but there are some known bugs in SMAPIv3 plugins currently.

So introduce a config file flag 'concurrent' for now, that defaults to false,
but that can be turned to 'true' for testing purposes.
When all SMAPIv3 concurrency bugs are believed to be fixed we can flip the default,
and eventually remove this flag once no more bugs are reported.
The configuration value is done as a global to simplify integrating intot he Lwt port,
instead of changing a lot of functions to thread through an argument.

This doesn't change the behaviour of xapi-storage-script in its default configuration.

Signed-off-by: Edwin Török <[email protected]>
  • Loading branch information
edwintorok committed Jul 16, 2024
1 parent b8b5fd0 commit f4e944f
Showing 1 changed file with 12 additions and 1 deletion.
13 changes: 12 additions & 1 deletion ocaml/xapi-storage-script/main.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1693,14 +1693,20 @@ let rec diff a b =
| a :: aa ->
if List.mem b a ~equal:String.( = ) then diff aa b else a :: diff aa b

(* default false due to bugs in SMAPIv3 plugins,
once they are fixed this should be set to true *)
let concurrent = ref false

let watch_volume_plugins ~volume_root ~switch_path ~pipe =
let create volume_plugin_name =
if Hashtbl.mem servers volume_plugin_name then
return ()
else (
info "Adding %s" volume_plugin_name ;
let volume_script_dir = Filename.concat volume_root volume_plugin_name in
Message_switch_async.Protocol_async.Server.listen
Message_switch_async.Protocol_async.Server.(
if !concurrent then listen_p else listen
)
~process:(process_smapiv2_requests (bind ~volume_script_dir))
~switch:switch_path
~queue:(Filename.basename volume_plugin_name)
Expand Down Expand Up @@ -1957,6 +1963,11 @@ let _ =
, (fun () -> string_of_bool !self_test_only)
, "Do only a self-test and exit"
)
; ( "concurrent"
, Arg.Set concurrent
, (fun () -> string_of_bool !concurrent)
, "Issue SMAPIv3 calls concurrently"
)
]
in
configure2 ~name:"xapi-script-storage" ~version:Xapi_version.version
Expand Down

0 comments on commit f4e944f

Please sign in to comment.