Skip to content

Commit

Permalink
CP-51352: Compare before setting a new value in last_active
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
GabrielBuica committed Sep 10, 2024
1 parent 88a47be commit 3ba6e6d
Showing 1 changed file with 27 additions and 3 deletions.
30 changes: 27 additions & 3 deletions ocaml/xapi/session_check.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 3ba6e6d

Please sign in to comment.