From 3ba6e6df2b321c0ff8323155532c84f325340208 Mon Sep 17 00:00:00 2001 From: Gabriel Buica Date: Tue, 3 Sep 2024 13:12:43 +0100 Subject: [PATCH 1/2] CP-51352: Compare before setting a new value in `last_active` Experimenting with xapi under different loads of `xe vm-list`s revealed that xapi often waits a considerable amount of time for the database lock. e.g. Attempting to start a VM under a constant load of 20 `xe vm-list`s can take up to 30s and the total amount of time that xapi requests would spend in `Db_actions.Session.set_last_active` would be close to 9s. It seems that each API call does a `Session_check.check` implicitly updating the last time the current session is been used. From what I gathered, this value is used for the garbage collection of session where the current default threshold value is 24 hours, `Xapi.globs.inactive_session_timeout`. To avoid holding the database lock to update the `last_active` field every time, this commit gets the current value of `last_active` and compares it with the current time. If the difference is greater than 10 minutes only then we set a new value in the field. Otherwise, we continue without writing in the database. This has the following improvement on the total time of `xe vm-start` under different loads: - 20 `xe vm-list`: - - Before: 30.670s | 30.086s | 30.958s | - - After: 28.185s | 28.383s | 28.909s | - 80 `xe vm-list`: - - Before: 2m38.425s | 2m43.539s | 2m39.045s | - - After: 1m46.266s | 1m49.327s | 1m36.825s | For smaller loads, the improvement is between 5-10%. Whereas for bigger loads, the improvement is much more visible. Signed-off-by: Gabriel Buica --- ocaml/xapi/session_check.ml | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/ocaml/xapi/session_check.ml b/ocaml/xapi/session_check.ml index 27812fc5244..63ac8fe6e95 100644 --- a/ocaml/xapi/session_check.ml +++ b/ocaml/xapi/session_check.ml @@ -53,9 +53,33 @@ let check ~intra_pool_only ~session_id ~action = if (not pool) && not (Pool_role.is_master ()) then raise Non_master_login_on_slave ; if Pool_role.is_master () then - Db_actions.DB_Action.Session.set_last_active ~__context - ~self:session_id - ~value:(Xapi_stdext_date.Date.of_float (Unix.time ())) + (* before updating the last_active field, check if the field has been + already updated recently. This avoids holding the database lock too often.*) + let n = Xapi_stdext_date.Date.now () in + let last_active = + Db_actions.DB_Action.Session.get_last_active ~__context + ~self:session_id + in + let ptime_now = Xapi_stdext_date.Date.to_ptime n in + let refresh_threshold = + let last_active_ptime = + Xapi_stdext_date.Date.to_ptime last_active + in + match + Ptime.add_span last_active_ptime (Ptime.Span.of_int_s 600) + with + | None -> + let err_msg = + "Can't add the configurable threshold of last active to \ + the current time." + in + raise Api_errors.(Server_error (internal_error, [err_msg])) + | Some ptime -> + ptime + in + if Ptime.is_later ptime_now ~than:refresh_threshold then + Db_actions.DB_Action.Session.set_last_active ~__context + ~self:session_id ~value:n with | Db_exn.DBCache_NotFound (_, _, reference) -> info From 67df15711c502a7f933fce7d46d1139b22779f87 Mon Sep 17 00:00:00 2001 From: Gabriel Buica Date: Tue, 3 Sep 2024 14:37:03 +0100 Subject: [PATCH 2/2] CP-51352: Configurable threshold for updating `last_active` Follow up to the previous commit, this makes the hardcoded threshold of 10 mins configurable from `xapi.conf`. This enables flexibilty in case different values are needed for updating the `last_active` field of a session more or less often. Signed-off-by: Gabriel Buica --- ocaml/xapi/session_check.ml | 3 ++- ocaml/xapi/xapi_globs.ml | 9 +++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/ocaml/xapi/session_check.ml b/ocaml/xapi/session_check.ml index 63ac8fe6e95..d30dbb6d4e3 100644 --- a/ocaml/xapi/session_check.ml +++ b/ocaml/xapi/session_check.ml @@ -66,7 +66,8 @@ let check ~intra_pool_only ~session_id ~action = Xapi_stdext_date.Date.to_ptime last_active in match - Ptime.add_span last_active_ptime (Ptime.Span.of_int_s 600) + Ptime.add_span last_active_ptime + !Xapi_globs.threshold_last_active with | None -> let err_msg = diff --git a/ocaml/xapi/xapi_globs.ml b/ocaml/xapi/xapi_globs.ml index 37c62e04e9f..56fbce47edd 100644 --- a/ocaml/xapi/xapi_globs.ml +++ b/ocaml/xapi/xapi_globs.ml @@ -712,6 +712,10 @@ let host_assumed_dead_interval = ref Mtime.Span.(10 * min) (* If a session has a last_active older than this we delete it *) let inactive_session_timeout = ref 86400. (* 24 hrs in seconds *) +(* If a session was refreshed more recently than threshold_last_active do not refresh it again. *) +let threshold_last_active = ref (Ptime.Span.of_int_s 600) +(* 10 min in seconds *) + let pending_task_timeout = ref 86400. (* 24 hrs in seconds *) let completed_task_timeout = ref 3900. (* 65 mins *) @@ -1631,6 +1635,11 @@ let other_options = , (fun () -> string_of_int !external_authentication_cache_size) , "Specify the maximum capacity of the external authentication cache" ) + ; ( "threshold_last_active" + , Arg.Int (fun t -> threshold_last_active := Ptime.Span.of_int_s t) + , (fun () -> Format.asprintf "%a" Ptime.Span.pp !threshold_last_active) + , "Specify the threshold below which we do not refresh the session" + ) ] (* The options can be set with the variable xapiflags in /etc/sysconfig/xapi.