From 5115fa139ae709bb5c808ea15d32df137e163e19 Mon Sep 17 00:00:00 2001 From: Gabriel Buica Date: Wed, 25 Sep 2024 10:10:10 +0100 Subject: [PATCH 1/8] CP-49064:`Tgroup` library Creates a new library `Tgroup`, that abstracts and manages groups of execution threads in xapi. When xapi is under load, all the threads need to share a single cpu in dom0 because of ocaml runtime single-cpu restrictions. This library is meant to orchestrate the threads in different priority groups. Signed-off-by: Gabriel Buica --- ocaml/libs/tgroup/dune | 3 + ocaml/libs/tgroup/tgroup.ml | 130 +++++++++++++++++++++++++++++++++++ ocaml/libs/tgroup/tgroup.mli | 70 +++++++++++++++++++ 3 files changed, 203 insertions(+) create mode 100644 ocaml/libs/tgroup/dune create mode 100644 ocaml/libs/tgroup/tgroup.ml create mode 100644 ocaml/libs/tgroup/tgroup.mli diff --git a/ocaml/libs/tgroup/dune b/ocaml/libs/tgroup/dune new file mode 100644 index 00000000000..d1cccfbe444 --- /dev/null +++ b/ocaml/libs/tgroup/dune @@ -0,0 +1,3 @@ +(library + (name tgroup) + (libraries xapi-log xapi-stdext-unix)) diff --git a/ocaml/libs/tgroup/tgroup.ml b/ocaml/libs/tgroup/tgroup.ml new file mode 100644 index 00000000000..c59729f69c5 --- /dev/null +++ b/ocaml/libs/tgroup/tgroup.ml @@ -0,0 +1,130 @@ +(* + * Copyright (C) Cloud Software Group + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published + * by the Free Software Foundation; version 2.1 only. with the special + * exception on linking described in file LICENSE. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + *) + +module D = Debug.Make (struct let name = __MODULE__ end) + +let ( // ) = Filename.concat + +module Group = struct + module Internal = struct + type t + + let name = "internal" + end + + module External = struct + type t + + let name = "external" + end + + module Host = struct + type t + + let name = "host" + end + + module SM = struct + type t + + let name = "SM" + end + + type _ group = + | Internal_Host_SM : (Internal.t * Host.t * SM.t) group + | EXTERNAL : External.t group + + type t = Group : 'a group -> t + + let all = [Group Internal_Host_SM; Group EXTERNAL] + + module Originator = struct + type t = Internal_Host_SM | EXTERNAL + + let of_string = function + | s + when String.equal + (String.lowercase_ascii SM.name) + (String.lowercase_ascii s) -> + Internal_Host_SM + | s + when String.equal + (String.lowercase_ascii External.name) + (String.lowercase_ascii s) -> + EXTERNAL + | _ -> + EXTERNAL + + let to_string = function + | Internal_Host_SM -> + SM.name + | EXTERNAL -> + External.name + end + + module Creator = struct + type t = { + user: string option + ; endpoint: string option + ; originator: Originator.t + } + + let make ?user ?endpoint originator = {originator; user; endpoint} + + let to_string c = + Printf.sprintf "Creator -> user:%s endpoint:%s originator:%s" + (Option.value c.user ~default:"") + (Option.value c.endpoint ~default:"") + (Originator.to_string c.originator) + end + + let of_originator = function + | Originator.Internal_Host_SM -> + Group Internal_Host_SM + | Originator.EXTERNAL -> + Group EXTERNAL + + let get_originator = function + | Group Internal_Host_SM -> + Originator.Internal_Host_SM + | Group EXTERNAL -> + Originator.EXTERNAL + + let of_creator creator = of_originator creator.Creator.originator + + let to_cgroup : type a. a group -> string = function + | Internal_Host_SM -> + Internal.name // Host.name // SM.name + | EXTERNAL -> + External.name +end + +module Cgroup = struct + type t = string + + let cgroup_dir = Atomic.make None + + let dir_of group : t option = + match group with + | Group.Group group -> + Option.map + (fun dir -> dir // Group.to_cgroup group) + (Atomic.get cgroup_dir) + + let init dir = + let () = Atomic.set cgroup_dir (Some dir) in + Group.all + |> List.filter_map dir_of + |> List.iter (fun dir -> Xapi_stdext_unix.Unixext.mkdir_rec dir 0o755) +end diff --git a/ocaml/libs/tgroup/tgroup.mli b/ocaml/libs/tgroup/tgroup.mli new file mode 100644 index 00000000000..ce87efc593c --- /dev/null +++ b/ocaml/libs/tgroup/tgroup.mli @@ -0,0 +1,70 @@ +(* + * Copyright (C) Cloud Software Group + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published + * by the Free Software Foundation; version 2.1 only. with the special + * exception on linking described in file LICENSE. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + *) + +(** [Group] module helps with the classification of different xapi execution + threads.*) +module Group : sig + (** Abstract type that represents a group of execution threads in xapi. Each + group corresponds to a Creator, and has a designated level of priority.*) + type t + + (** Generic representation of different xapi threads originators. *) + module Originator : sig + (** Type that represents different originators of xapi threads. *) + type t = Internal_Host_SM | EXTERNAL + + val of_string : string -> t + (** [of_string s] creates an originator from a string [s]. + + e.g create an originator based on a http header. *) + + val to_string : t -> string + (** [to_string o] converts an originator [o] to its string representation.*) + end + + (** Generic representation of different xapi threads creators. *) + module Creator : sig + (** Abstract type that represents different creators of xapi threads.*) + type t + + val make : ?user:string -> ?endpoint:string -> Originator.t -> t + (** [make o] creates a creator type based on a given originator [o].*) + + val to_string : t -> string + (** [to_string c] converts a creator [c] to its string representation.*) + end + + val get_originator : t -> Originator.t + (** [get_originator group] returns the originator that maps to group [group].*) + + val of_creator : Creator.t -> t + (** [of_creator c] returns the corresponding group based on the creator [c].*) +end + +(** [Cgroup] module encapsulates different function for managing the cgroups +corresponding with [Groups].*) +module Cgroup : sig + (** Represents one of the children of the cgroup directory.*) + type t = string + + val dir_of : Group.t -> t option + (** [dir_of group] returns the full path of the cgroup directory corresponding + to the group [group] as [Some dir]. + + Returns [None] if [init dir] has not been called. *) + + val init : string -> unit + (** [init dir] initializes the hierachy of cgroups associated to all [Group.t] + types under the directory [dir].*) +end From ce7de908fd398a96521a8a95c5c4ed88bd183c25 Mon Sep 17 00:00:00 2001 From: Gabriel Buica Date: Wed, 25 Sep 2024 14:46:55 +0100 Subject: [PATCH 2/8] CP-51493: Add `set_cgroup` `set_cgroup` adds the functionality of adding the current thread in a cgroup based on its creator. Signed-off-by: Gabriel Buica --- ocaml/libs/tgroup/dune | 2 +- ocaml/libs/tgroup/tgroup.ml | 34 ++++++++++++++++++++++++++++++++++ ocaml/libs/tgroup/tgroup.mli | 4 ++++ 3 files changed, 39 insertions(+), 1 deletion(-) diff --git a/ocaml/libs/tgroup/dune b/ocaml/libs/tgroup/dune index d1cccfbe444..025a5adc891 100644 --- a/ocaml/libs/tgroup/dune +++ b/ocaml/libs/tgroup/dune @@ -1,3 +1,3 @@ (library (name tgroup) - (libraries xapi-log xapi-stdext-unix)) + (libraries xapi-log xapi-stdext-unix xapi-stdext-pervasives)) diff --git a/ocaml/libs/tgroup/tgroup.ml b/ocaml/libs/tgroup/tgroup.ml index c59729f69c5..b4c87b087b9 100644 --- a/ocaml/libs/tgroup/tgroup.ml +++ b/ocaml/libs/tgroup/tgroup.ml @@ -14,6 +14,8 @@ module D = Debug.Make (struct let name = __MODULE__ end) +open D + let ( // ) = Filename.concat module Group = struct @@ -127,4 +129,36 @@ module Cgroup = struct Group.all |> List.filter_map dir_of |> List.iter (fun dir -> Xapi_stdext_unix.Unixext.mkdir_rec dir 0o755) + + let write_cur_tid_to_cgroup_file filename = + try + let perms = 0o640 in + let mode = [Unix.O_WRONLY; Unix.O_CREAT; Unix.O_TRUNC] in + Xapi_stdext_unix.Unixext.with_file filename mode perms @@ fun fd -> + (* Writing 0 to the task file will automatically transform in writing + the current caller tid to the file. + + Writing 0 to the processes file will automatically write the caller's + pid to file. *) + let buf = "0\n" in + let len = String.length buf in + if Unix.write fd (Bytes.unsafe_of_string buf) 0 len <> len then + warn "writing current tid to %s failed" filename + with exn -> + warn "writing current tid to %s failed with exception: %s" filename + (Printexc.to_string exn) + + let attach_task group = + let tasks_file = dir_of group // "tasks" in + write_cur_tid_to_cgroup_file tasks_file + + let set_cur_cgroup ~originator = + match originator with + | Group.Originator.Internal_Host_SM -> + attach_task (Group Internal_Host_SM) + | Group.Originator.EXTERNAL -> + attach_task (Group EXTERNAL) + + let set_cgroup creator = + set_cur_cgroup ~originator:creator.Group.Creator.originator end diff --git a/ocaml/libs/tgroup/tgroup.mli b/ocaml/libs/tgroup/tgroup.mli index ce87efc593c..dc39735425b 100644 --- a/ocaml/libs/tgroup/tgroup.mli +++ b/ocaml/libs/tgroup/tgroup.mli @@ -67,4 +67,8 @@ module Cgroup : sig val init : string -> unit (** [init dir] initializes the hierachy of cgroups associated to all [Group.t] types under the directory [dir].*) + + val set_cgroup : Group.Creator.t -> unit + (** [set_cgroup c] sets the current xapi thread in a cgroup based on the + creator [c].*) end From 0714ce27235c341919cff512ea8cefb6c9802668 Mon Sep 17 00:00:00 2001 From: Gabriel Buica Date: Wed, 25 Sep 2024 15:39:17 +0100 Subject: [PATCH 3/8] CP-51488: Set `tgroup` based on request header. Add functionality of setting the tgroup based on a http header named `originator`. Signed-off-by: Gabriel Buica --- ocaml/libs/tgroup/dune | 2 +- ocaml/libs/tgroup/tgroup.ml | 11 +++++++++++ ocaml/libs/tgroup/tgroup.mli | 4 ++++ 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/ocaml/libs/tgroup/dune b/ocaml/libs/tgroup/dune index 025a5adc891..d1cccfbe444 100644 --- a/ocaml/libs/tgroup/dune +++ b/ocaml/libs/tgroup/dune @@ -1,3 +1,3 @@ (library (name tgroup) - (libraries xapi-log xapi-stdext-unix xapi-stdext-pervasives)) + (libraries xapi-log xapi-stdext-unix)) diff --git a/ocaml/libs/tgroup/tgroup.ml b/ocaml/libs/tgroup/tgroup.ml index b4c87b087b9..24d8cd6e389 100644 --- a/ocaml/libs/tgroup/tgroup.ml +++ b/ocaml/libs/tgroup/tgroup.ml @@ -162,3 +162,14 @@ module Cgroup = struct let set_cgroup creator = set_cur_cgroup ~originator:creator.Group.Creator.originator end + +let of_originator originator = + originator |> Group.Creator.make |> Cgroup.set_cgroup + +let of_req_originator originator = + try + originator + |> Option.value ~default:Group.Originator.(to_string EXTERNAL) + |> Group.Originator.of_string + |> of_originator + with _ -> () diff --git a/ocaml/libs/tgroup/tgroup.mli b/ocaml/libs/tgroup/tgroup.mli index dc39735425b..e1d5c7f0b85 100644 --- a/ocaml/libs/tgroup/tgroup.mli +++ b/ocaml/libs/tgroup/tgroup.mli @@ -72,3 +72,7 @@ module Cgroup : sig (** [set_cgroup c] sets the current xapi thread in a cgroup based on the creator [c].*) end + +val of_req_originator : string option -> unit +(** [of_req_originator o] same as [of_originator] but it classifies based on the +http request header.*) From 3d822a71c7a0ed9baa6365773201b8ea88154541 Mon Sep 17 00:00:00 2001 From: Gabriel Buica Date: Wed, 2 Oct 2024 13:58:08 +0100 Subject: [PATCH 4/8] CP-49064: Init cgroups at xapi startup Signed-off-by: Gabriel Buica --- dune-project | 8 ++++++++ ocaml/libs/tgroup/dune | 1 + ocaml/xapi/dune | 2 ++ ocaml/xapi/xapi.ml | 4 ++++ ocaml/xapi/xapi_globs.ml | 3 +++ tgroup.opam | 28 ++++++++++++++++++++++++++++ xapi.opam | 1 + 7 files changed, 47 insertions(+) create mode 100644 tgroup.opam diff --git a/dune-project b/dune-project index 15ff4a5fbfa..649162d0fc1 100644 --- a/dune-project +++ b/dune-project @@ -37,6 +37,13 @@ ) ) +(package + (name tgroup) + (depends + xapi-log + xapi-stdext-unix) +) + (package (name xml-light2) ) @@ -373,6 +380,7 @@ tar tar-unix uri + tgroup (uuid (= :version)) uutf uuidm diff --git a/ocaml/libs/tgroup/dune b/ocaml/libs/tgroup/dune index d1cccfbe444..40b75ad1bbd 100644 --- a/ocaml/libs/tgroup/dune +++ b/ocaml/libs/tgroup/dune @@ -1,3 +1,4 @@ (library (name tgroup) + (public_name tgroup) (libraries xapi-log xapi-stdext-unix)) diff --git a/ocaml/xapi/dune b/ocaml/xapi/dune index 9f3e5f825fa..048bd4963f9 100644 --- a/ocaml/xapi/dune +++ b/ocaml/xapi/dune @@ -151,6 +151,7 @@ tapctl tar tar-unix + tgroup threads.posix tracing unixpwd @@ -237,6 +238,7 @@ rpclib.json rpclib.xml stunnel + tgroup threads.posix tracing xapi-backtrace diff --git a/ocaml/xapi/xapi.ml b/ocaml/xapi/xapi.ml index ca87e740efb..6c2475c7929 100644 --- a/ocaml/xapi/xapi.ml +++ b/ocaml/xapi/xapi.ml @@ -1058,6 +1058,10 @@ let server_init () = ; ("Initialising random number generator", [], random_setup) ; ("Initialise TLS verification", [], init_tls_verification) ; ("Running startup check", [], startup_check) + ; ( "Initialize cgroups via tgroup" + , [] + , fun () -> Tgroup.Cgroup.init Xapi_globs.xapi_requests_cgroup + ) ; ( "Registering SMAPIv1 plugins" , [Startup.OnlyMaster] , Sm.register ~__context diff --git a/ocaml/xapi/xapi_globs.ml b/ocaml/xapi/xapi_globs.ml index d59af9e2e49..f2912ab1bb6 100644 --- a/ocaml/xapi/xapi_globs.ml +++ b/ocaml/xapi/xapi_globs.ml @@ -1059,6 +1059,9 @@ let disable_webserver = ref false let test_open = ref 0 +let xapi_requests_cgroup = + "/sys/fs/cgroup/cpu/control.slice/xapi.service/request" + let xapi_globs_spec = [ ( "master_connection_reset_timeout" diff --git a/tgroup.opam b/tgroup.opam new file mode 100644 index 00000000000..423b4628877 --- /dev/null +++ b/tgroup.opam @@ -0,0 +1,28 @@ +# This file is generated by dune, edit dune-project instead +opam-version: "2.0" +maintainer: ["Xapi project maintainers"] +authors: ["xen-api@lists.xen.org"] +license: "LGPL-2.1-only WITH OCaml-LGPL-linking-exception" +homepage: "https://xapi-project.github.io/" +bug-reports: "https://github.com/xapi-project/xen-api/issues" +depends: [ + "dune" {>= "3.15"} + "xapi-log" + "xapi-stdext-unix" + "odoc" {with-doc} +] +build: [ + ["dune" "subst"] {dev} + [ + "dune" + "build" + "-p" + name + "-j" + jobs + "@install" + "@runtest" {with-test} + "@doc" {with-doc} + ] +] +dev-repo: "git+https://github.com/xapi-project/xen-api.git" diff --git a/xapi.opam b/xapi.opam index 098d8463442..e9dce9e47f5 100644 --- a/xapi.opam +++ b/xapi.opam @@ -63,6 +63,7 @@ depends: [ "tar" "tar-unix" "uri" + "tgroup" "uuid" {= version} "uutf" "uuidm" From e90b32ca2aa5e3532dc80e6271923046e174138d Mon Sep 17 00:00:00 2001 From: Gabriel Buica Date: Mon, 28 Oct 2024 11:56:41 +0000 Subject: [PATCH 5/8] CP-50537: Always reset `_extra_headers` when making a connection. Clears the `extra_headers` of `UDSTransport` instance when making a connection. Previously, this was done only when tracing was enabled inside the `with_tracecontext` method to avoid the header duplication when `make_connection` was used multiple times. Currently, there is not other use of `add_extra_headers` or other update to the `_extra_headers`, making it safe to clear it when we make a new connection. (`xmlrpclib.Transport` updates the `_extra_headers` attribute only inside `make_connection` method but we override this method with our own for `UDSTransport`.) Signed-off-by: Gabriel Buica --- python3/examples/XenAPI/XenAPI.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/python3/examples/XenAPI/XenAPI.py b/python3/examples/XenAPI/XenAPI.py index e37f8813b6e..08f61749142 100644 --- a/python3/examples/XenAPI/XenAPI.py +++ b/python3/examples/XenAPI/XenAPI.py @@ -106,6 +106,7 @@ def connect(self): class UDSTransport(xmlrpclib.Transport): def add_extra_header(self, key, value): self._extra_headers += [ (key,value) ] + def with_tracecontext(self): if otel: headers = {} @@ -114,10 +115,14 @@ def with_tracecontext(self): # pylint: disable=possibly-used-before-assignment propagators = propagate.get_global_textmap() propagators.inject(headers, ctx) - self._extra_headers = [] + for k, v in headers.items(): self.add_extra_header(k, v) + def make_connection(self, host): + # clear the extra headers when making a new connection. This makes sure + # headers such as "traceparent" do not get duplicated. + self._extra_headers = [] self.with_tracecontext() # compatibility with parent xmlrpclib.Transport HTTP/1.1 support From 4f5dbb5e6d24e31108a1df860bf4aba665ade690 Mon Sep 17 00:00:00 2001 From: Gabriel Buica Date: Wed, 23 Oct 2024 10:51:45 +0100 Subject: [PATCH 6/8] CP-50537: Propagate originator as a http request header XenAPI.py now passes an additional originator header when making requests to xapi, if the "ORIGINATOR" env var is present. Sm_exec now passes an env var, "ORIGINATOR=SM". To classify the threads correctly, we first need to determine the requests originators. This commit makes it possibly to explicitly state the originators as headers. Signed-off-by: Gabriel Buica --- ocaml/libs/http-lib/http.ml | 10 ++++++++++ ocaml/libs/http-lib/http.mli | 2 ++ ocaml/xapi/sm_exec.ml | 11 +++++++++-- ocaml/xapi/xapi_observer_components.ml | 8 ++++---- ocaml/xapi/xapi_observer_components.mli | 3 ++- python3/examples/XenAPI/XenAPI.py | 7 +++++++ 6 files changed, 34 insertions(+), 7 deletions(-) diff --git a/ocaml/libs/http-lib/http.ml b/ocaml/libs/http-lib/http.ml index a19745576ce..8f352eb9237 100644 --- a/ocaml/libs/http-lib/http.ml +++ b/ocaml/libs/http-lib/http.ml @@ -132,6 +132,8 @@ module Hdr = struct let location = "location" + let originator = "originator" + let traceparent = "traceparent" let hsts = "strict-transport-security" @@ -688,6 +690,14 @@ module Request = struct let frame_header = if x.frame then make_frame_header headers else "" in frame_header ^ headers ^ body + let with_originator_of req f = + Option.iter + (fun req -> + let originator = List.assoc_opt Hdr.originator req.additional_headers in + f originator + ) + req + let traceparent_of req = let open Tracing in let ( let* ) = Option.bind in diff --git a/ocaml/libs/http-lib/http.mli b/ocaml/libs/http-lib/http.mli index 3fbae8e4c6f..21fc00a8ee6 100644 --- a/ocaml/libs/http-lib/http.mli +++ b/ocaml/libs/http-lib/http.mli @@ -129,6 +129,8 @@ module Request : sig val to_wire_string : t -> string (** [to_wire_string t] returns a string which could be sent to a server *) + val with_originator_of : t option -> (string option -> unit) -> unit + val traceparent_of : t -> Tracing.Span.t option val with_tracing : diff --git a/ocaml/xapi/sm_exec.ml b/ocaml/xapi/sm_exec.ml index 28cdd11e07b..c95c3bcb28e 100644 --- a/ocaml/xapi/sm_exec.ml +++ b/ocaml/xapi/sm_exec.ml @@ -38,6 +38,13 @@ let with_dbg ~name ~dbg f = (*********************************************************************************************) (* Random utility functions *) +let env_vars = + Array.concat + [ + Forkhelpers.default_path_env_pair + ; Env_record.to_string_array [Env_record.pair ("ORIGINATOR", "SM")] + ] + type call = { (* All calls are performed by a specific Host with a special Session and device_config *) host_ref: API.ref_host @@ -355,9 +362,9 @@ let exec_xmlrpc ~dbg ?context:_ ?(needs_session = true) (driver : string) let env, exe, args = match Xapi_observer_components.is_smapi_enabled () with | false -> - (None, exe, args) + (Some env_vars, exe, args) | true -> - Xapi_observer_components.env_exe_args_of + Xapi_observer_components.env_exe_args_of ~env_vars ~component:Xapi_observer_components.SMApi ~exe ~args in Forkhelpers.execute_command_get_output ?tracing:di.tracing ?env diff --git a/ocaml/xapi/xapi_observer_components.ml b/ocaml/xapi/xapi_observer_components.ml index d3e0587b143..0b3b884f465 100644 --- a/ocaml/xapi/xapi_observer_components.ml +++ b/ocaml/xapi/xapi_observer_components.ml @@ -101,12 +101,12 @@ let ( // ) = Filename.concat let dir_name_of_component component = Xapi_globs.observer_config_dir // to_string component // "enabled" -let env_exe_args_of ~component ~exe ~args = +let env_exe_args_of ~env_vars ~component ~exe ~args = let dir_name_value = Filename.quote (dir_name_of_component component) in - let env_vars = + let new_env_vars = Array.concat [ - Forkhelpers.default_path_env_pair + env_vars ; Env_record.to_string_array [ Env_record.pair ("OBSERVER_CONFIG_DIR", dir_name_value) @@ -116,4 +116,4 @@ let env_exe_args_of ~component ~exe ~args = in let args = "-m" :: "observer" :: exe :: args in let new_exe = Xapi_globs.python3_path in - (Some env_vars, new_exe, args) + (Some new_env_vars, new_exe, args) diff --git a/ocaml/xapi/xapi_observer_components.mli b/ocaml/xapi/xapi_observer_components.mli index 55bdf7e7f05..9e046dddaf3 100644 --- a/ocaml/xapi/xapi_observer_components.mli +++ b/ocaml/xapi/xapi_observer_components.mli @@ -63,7 +63,8 @@ val dir_name_of_component : t -> string *) val env_exe_args_of : - component:t + env_vars:string array + -> component:t -> exe:string -> args:string list -> string array option * string * string list diff --git a/python3/examples/XenAPI/XenAPI.py b/python3/examples/XenAPI/XenAPI.py index 08f61749142..012dcf40de7 100644 --- a/python3/examples/XenAPI/XenAPI.py +++ b/python3/examples/XenAPI/XenAPI.py @@ -119,11 +119,18 @@ def with_tracecontext(self): for k, v in headers.items(): self.add_extra_header(k, v) + def with_originator(self): + originator_k = "ORIGINATOR" + originator_v = os.getenv(originator_k, None) + if originator_v: + self.add_extra_header(originator_k.lower(), originator_v) + def make_connection(self, host): # clear the extra headers when making a new connection. This makes sure # headers such as "traceparent" do not get duplicated. self._extra_headers = [] self.with_tracecontext() + self.with_originator() # compatibility with parent xmlrpclib.Transport HTTP/1.1 support if self._connection and host == self._connection[0]: From 13cff9f924a2b2f400dd8bce14927467e42022f9 Mon Sep 17 00:00:00 2001 From: Gabriel Buica Date: Tue, 22 Oct 2024 14:57:44 +0100 Subject: [PATCH 7/8] CP-51489: Classify threads based on http requests. For now, the thread executing `Xapi.server_init` and it's children are classified as External. The only excception are http requests that come through the smapi internally. If those contain the originator header with the value set as "sm", the thread executing the request will be classified as internal. This represents the first phase of classifing xapi threads as internal vs external. Signed-off-by: Gabriel Buica --- dune-project | 1 + http-lib.opam | 1 + ocaml/libs/http-lib/dune | 1 + ocaml/libs/http-lib/http_svr.ml | 2 +- ocaml/libs/tgroup/tgroup.ml | 21 +++++++++++++-------- 5 files changed, 17 insertions(+), 9 deletions(-) diff --git a/dune-project b/dune-project index 649162d0fc1..e69a04e745a 100644 --- a/dune-project +++ b/dune-project @@ -593,6 +593,7 @@ This package provides an Lwt compatible interface to the library.") (safe-resources(= :version)) sha (stunnel (= :version)) + tgroup uri (uuid (= :version)) xapi-backtrace diff --git a/http-lib.opam b/http-lib.opam index df1b7735eb7..ea91e9c942d 100644 --- a/http-lib.opam +++ b/http-lib.opam @@ -22,6 +22,7 @@ depends: [ "safe-resources" {= version} "sha" "stunnel" {= version} + "tgroup" "uri" "uuid" {= version} "xapi-backtrace" diff --git a/ocaml/libs/http-lib/dune b/ocaml/libs/http-lib/dune index 2990fda2453..c74d6a52e32 100644 --- a/ocaml/libs/http-lib/dune +++ b/ocaml/libs/http-lib/dune @@ -44,6 +44,7 @@ http_lib ipaddr polly + tgroup threads.posix tracing uri diff --git a/ocaml/libs/http-lib/http_svr.ml b/ocaml/libs/http-lib/http_svr.ml index 54a8b96ba73..d412b9d025b 100644 --- a/ocaml/libs/http-lib/http_svr.ml +++ b/ocaml/libs/http-lib/http_svr.ml @@ -560,7 +560,7 @@ let handle_connection ~header_read_timeout ~header_total_timeout read_request ?proxy_seen ~read_timeout ~total_timeout ~max_length:max_header_length ss in - + Http.Request.with_originator_of req Tgroup.of_req_originator ; (* 2. now we attempt to process the request *) let finished = Option.fold ~none:true diff --git a/ocaml/libs/tgroup/tgroup.ml b/ocaml/libs/tgroup/tgroup.ml index 24d8cd6e389..557b66bc1c9 100644 --- a/ocaml/libs/tgroup/tgroup.ml +++ b/ocaml/libs/tgroup/tgroup.ml @@ -124,12 +124,6 @@ module Cgroup = struct (fun dir -> dir // Group.to_cgroup group) (Atomic.get cgroup_dir) - let init dir = - let () = Atomic.set cgroup_dir (Some dir) in - Group.all - |> List.filter_map dir_of - |> List.iter (fun dir -> Xapi_stdext_unix.Unixext.mkdir_rec dir 0o755) - let write_cur_tid_to_cgroup_file filename = try let perms = 0o640 in @@ -149,8 +143,12 @@ module Cgroup = struct (Printexc.to_string exn) let attach_task group = - let tasks_file = dir_of group // "tasks" in - write_cur_tid_to_cgroup_file tasks_file + Option.iter + (fun dir -> + let tasks_file = dir // "tasks" in + write_cur_tid_to_cgroup_file tasks_file + ) + (dir_of group) let set_cur_cgroup ~originator = match originator with @@ -161,6 +159,13 @@ module Cgroup = struct let set_cgroup creator = set_cur_cgroup ~originator:creator.Group.Creator.originator + + let init dir = + let () = Atomic.set cgroup_dir (Some dir) in + Group.all + |> List.filter_map dir_of + |> List.iter (fun dir -> Xapi_stdext_unix.Unixext.mkdir_rec dir 0o755) ; + set_cur_cgroup ~originator:Group.Originator.EXTERNAL end let of_originator originator = From efaf3f0c86cb075c9e2e6d546329b3625d0a55bf Mon Sep 17 00:00:00 2001 From: Gabriel Buica Date: Mon, 4 Nov 2024 16:07:26 +0000 Subject: [PATCH 8/8] CP-50537: Add a guard in `xapi_globs`, `Xapi_globs.tgroups_enabled`. Adds a configurable variable in `xapi_globs`, `tgroups_enabled` that is meant to ask a guard for tgroup classification of the threads. If the guard is `false` all Tgroups functionality should act as a no op. For instance, adding the line: tgroups-enabled = false will result in the thread classification being skipped. Signed-off-by: Gabriel Buica --- ocaml/libs/http-lib/http_svr.ml | 2 ++ ocaml/libs/tgroup/tgroup.ml | 16 ++++++++++------ ocaml/xapi/xapi.ml | 4 +++- ocaml/xapi/xapi_globs.ml | 7 +++++++ 4 files changed, 22 insertions(+), 7 deletions(-) diff --git a/ocaml/libs/http-lib/http_svr.ml b/ocaml/libs/http-lib/http_svr.ml index d412b9d025b..64c9c929177 100644 --- a/ocaml/libs/http-lib/http_svr.ml +++ b/ocaml/libs/http-lib/http_svr.ml @@ -560,7 +560,9 @@ let handle_connection ~header_read_timeout ~header_total_timeout read_request ?proxy_seen ~read_timeout ~total_timeout ~max_length:max_header_length ss in + Http.Request.with_originator_of req Tgroup.of_req_originator ; + (* 2. now we attempt to process the request *) let finished = Option.fold ~none:true diff --git a/ocaml/libs/tgroup/tgroup.ml b/ocaml/libs/tgroup/tgroup.ml index 557b66bc1c9..a0639974670 100644 --- a/ocaml/libs/tgroup/tgroup.ml +++ b/ocaml/libs/tgroup/tgroup.ml @@ -172,9 +172,13 @@ let of_originator originator = originator |> Group.Creator.make |> Cgroup.set_cgroup let of_req_originator originator = - try - originator - |> Option.value ~default:Group.Originator.(to_string EXTERNAL) - |> Group.Originator.of_string - |> of_originator - with _ -> () + Option.iter + (fun _ -> + try + originator + |> Option.value ~default:Group.Originator.(to_string EXTERNAL) + |> Group.Originator.of_string + |> of_originator + with _ -> () + ) + (Atomic.get Cgroup.cgroup_dir) diff --git a/ocaml/xapi/xapi.ml b/ocaml/xapi/xapi.ml index 6c2475c7929..9b87f1de6b5 100644 --- a/ocaml/xapi/xapi.ml +++ b/ocaml/xapi/xapi.ml @@ -1060,7 +1060,9 @@ let server_init () = ; ("Running startup check", [], startup_check) ; ( "Initialize cgroups via tgroup" , [] - , fun () -> Tgroup.Cgroup.init Xapi_globs.xapi_requests_cgroup + , fun () -> + if !Xapi_globs.tgroups_enabled then + Tgroup.Cgroup.init Xapi_globs.xapi_requests_cgroup ) ; ( "Registering SMAPIv1 plugins" , [Startup.OnlyMaster] diff --git a/ocaml/xapi/xapi_globs.ml b/ocaml/xapi/xapi_globs.ml index f2912ab1bb6..ef3c5ce66a9 100644 --- a/ocaml/xapi/xapi_globs.ml +++ b/ocaml/xapi/xapi_globs.ml @@ -1059,6 +1059,8 @@ let disable_webserver = ref false let test_open = ref 0 +let tgroups_enabled = ref false + let xapi_requests_cgroup = "/sys/fs/cgroup/cpu/control.slice/xapi.service/request" @@ -1624,6 +1626,11 @@ let other_options = , (fun () -> string_of_bool !disable_webserver) , "Disable the host webserver" ) + ; ( "tgroups-enabled" + , Arg.Set tgroups_enabled + , (fun () -> string_of_bool !tgroups_enabled) + , "Turn on tgroups classification" + ) ] (* The options can be set with the variable xapiflags in /etc/sysconfig/xapi.