From 8893b800e07b351761ee2b00c66f0926973a4cc8 Mon Sep 17 00:00:00 2001 From: Norman Feske Date: Thu, 20 Apr 2023 12:44:40 +0200 Subject: [PATCH] depot_deploy: report only on state changes This patch mitigates potential busy feedback effects when evaluating the reports produced by 'depot_deploy' in a closed control loop. Reports are now generated only if the deployment state has changed. Issue #4818 --- repos/gems/src/app/depot_deploy/child.h | 67 ++++--- repos/gems/src/app/depot_deploy/children.h | 51 ++++- repos/gems/src/app/depot_deploy/main.cc | 194 ++++++++++++-------- repos/gems/src/app/sculpt_manager/deploy.cc | 110 ++++++----- 4 files changed, 271 insertions(+), 151 deletions(-) diff --git a/repos/gems/src/app/depot_deploy/child.h b/repos/gems/src/app/depot_deploy/child.h index ee855cd432..8fae221506 100644 --- a/repos/gems/src/app/depot_deploy/child.h +++ b/repos/gems/src/app/depot_deploy/child.h @@ -115,7 +115,9 @@ class Depot_deploy::Child : public List_model::Element /* * Set if the depot query for the child's blueprint failed. */ - bool _pkg_incomplete = false; + enum class State { UNKNOWN, PKG_INCOMPLETE, PKG_COMPLETE }; + + State _state = State::UNKNOWN; bool _configured() const { @@ -159,10 +161,13 @@ class Depot_deploy::Child : public List_model::Element Name name() const { return _name; } - void apply_config(Xml_node start_node) + /* + * \return true if config had an effect on the child's state + */ + bool apply_config(Xml_node start_node) { if (!start_node.differs_from(_start_xml->xml())) - return; + return false; Archive::Path const old_pkg_path = _config_pkg_path(); @@ -177,14 +182,21 @@ class Depot_deploy::Child : public List_model::Element _pkg_xml.destruct(); /* reset error state, attempt to obtain the blueprint again */ - _pkg_incomplete = false; + _state = State::UNKNOWN; } + return true; } - void apply_blueprint(Xml_node pkg) + /* + * \return true if bluerprint had an effect on the child + */ + bool apply_blueprint(Xml_node pkg) { + if (_state == State::PKG_COMPLETE) + return false; + if (pkg.attribute_value("path", Archive::Path()) != _blueprint_pkg_path) - return; + return false; /* check for the completeness of all ROM ingredients */ bool any_rom_missing = false; @@ -200,12 +212,13 @@ class Depot_deploy::Child : public List_model::Element }); if (any_rom_missing) { - _pkg_incomplete = true; - return; + State const orig_state = _state; + _state = State::PKG_INCOMPLETE; + return (orig_state != _state); } /* package was missing but is installed now */ - _pkg_incomplete = false; + _state = State::PKG_COMPLETE; Xml_node const runtime = pkg.sub_node("runtime"); @@ -218,22 +231,26 @@ class Depot_deploy::Child : public List_model::Element /* keep copy of the blueprint info */ _pkg_xml.construct(_alloc, pkg); + + return true; } - void apply_launcher(Launcher_name const &name, Xml_node launcher) + bool apply_launcher(Launcher_name const &name, Xml_node launcher) { if (!_defined_by_launcher()) - return; + return false; if (_launcher_name() != name) - return; + return false; if (_launcher_xml.constructed() && !launcher.differs_from(_launcher_xml->xml())) - return; + return false; _launcher_xml.construct(_alloc, launcher); _blueprint_pkg_path = _config_pkg_path(); + + return true; } /* @@ -266,19 +283,25 @@ class Depot_deploy::Child : public List_model::Element fn(_start_xml->xml(), launcher_xml, _name); } - void mark_as_incomplete(Xml_node missing) + /* + * \return true if the call had an effect on the child + */ + bool mark_as_incomplete(Xml_node missing) { /* print error message only once */ - if(_pkg_incomplete) - return; + if(_state == State::PKG_INCOMPLETE) + return false; Archive::Path const path = missing.attribute_value("path", Archive::Path()); if (path != _blueprint_pkg_path) - return; + return false; log(path, " incomplete or missing"); - _pkg_incomplete = true; + State const orig_state = _state; + _state = State::PKG_INCOMPLETE; + + return (orig_state != _state); } /** @@ -286,8 +309,8 @@ class Depot_deploy::Child : public List_model::Element */ void reset_incomplete() { - if (_pkg_incomplete) { - _pkg_incomplete = false; + if (_state == State::PKG_INCOMPLETE) { + _state = State::UNKNOWN; _pkg_xml.destruct(); } } @@ -333,7 +356,7 @@ class Depot_deploy::Child : public List_model::Element template void with_missing_pkg_path(FN const &fn) const { - if (_pkg_incomplete) + if (_state == State::PKG_INCOMPLETE) fn(_config_pkg_path()); } @@ -348,7 +371,7 @@ class Depot_deploy::Child : public List_model::Element xml.attribute("source", "no"); }); }); } - bool incomplete() const { return _pkg_incomplete; } + bool incomplete() const { return _state == State::PKG_INCOMPLETE; } }; diff --git a/repos/gems/src/app/depot_deploy/children.h b/repos/gems/src/app/depot_deploy/children.h index 0a9ef2e773..b22c582995 100644 --- a/repos/gems/src/app/depot_deploy/children.h +++ b/repos/gems/src/app/depot_deploy/children.h @@ -38,16 +38,27 @@ class Depot_deploy::Children { Allocator &_alloc; + unsigned num_changes = 0; + Model_update_policy(Allocator &alloc) : _alloc(alloc) { } - void destroy_element(Child &c) { destroy(_alloc, &c); } + void destroy_element(Child &c) + { + num_changes++; + destroy(_alloc, &c); + } Child &create_element(Xml_node node) { + num_changes++; return *new (_alloc) Child(_alloc, node); } - void update_element(Child &c, Xml_node node) { c.apply_config(node); } + void update_element(Child &child, Xml_node node) + { + if (child.apply_config(node)) + num_changes++; + } static bool element_matches_xml_node(Child const &child, Xml_node node) { @@ -62,26 +73,50 @@ class Depot_deploy::Children Children(Allocator &alloc) : _alloc(alloc) { } - void apply_config(Xml_node config) + /* + * \return true if config had any effect + */ + bool apply_config(Xml_node config) { + unsigned const orig_num_changes = _model_update_policy.num_changes; + _children.update_from_xml(_model_update_policy, config); + + return (orig_num_changes != _model_update_policy.num_changes); } - void apply_launcher(Child::Launcher_name const &name, Xml_node launcher) + /* + * \return true if launcher had any effect + */ + bool apply_launcher(Child::Launcher_name const &name, Xml_node launcher) { + bool any_child_changed = false; + _children.for_each([&] (Child &child) { - child.apply_launcher(name, launcher); }); + if (child.apply_launcher(name, launcher)) + any_child_changed = true; }); + + return any_child_changed; } - void apply_blueprint(Xml_node blueprint) + /* + * \return true if blueprint had an effect on any child + */ + bool apply_blueprint(Xml_node const &blueprint) { + bool any_child_changed = false; + blueprint.for_each_sub_node("pkg", [&] (Xml_node pkg) { _children.for_each([&] (Child &child) { - child.apply_blueprint(pkg); }); }); + if (child.apply_blueprint(pkg)) + any_child_changed = true; }); }); blueprint.for_each_sub_node("missing", [&] (Xml_node missing) { _children.for_each([&] (Child &child) { - child.mark_as_incomplete(missing); }); }); + if (child.mark_as_incomplete(missing)) + any_child_changed = true; }); }); + + return any_child_changed; } /* diff --git a/repos/gems/src/app/depot_deploy/main.cc b/repos/gems/src/app/depot_deploy/main.cc index 4d4fda8a20..3dea494135 100644 --- a/repos/gems/src/app/depot_deploy/main.cc +++ b/repos/gems/src/app/depot_deploy/main.cc @@ -44,106 +44,78 @@ struct Depot_deploy::Main Signal_handler
_config_handler { _env.ep(), *this, &Main::_handle_config }; - typedef String<128> Name; + using Name = String<128>; + using Prio_levels = Child::Prio_levels; + using Arch = String<16>; + + Prio_levels _prio_levels { }; + Arch _arch { }; void _handle_config() { _config.update(); _blueprint.update(); + /* + * Capture original state, used to detect if the config has any effect + */ + struct Attributes + { + bool state_reporter_constructed; + Prio_levels prio_levels; + Arch arch; + + bool operator != (Attributes const &other) const + { + return other.state_reporter_constructed != state_reporter_constructed + || other.prio_levels.value != prio_levels.value + || other.arch != arch; + } + }; + + auto curr_attributes = [&] { + return Attributes { + .state_reporter_constructed = _state_reporter.constructed(), + .prio_levels = _prio_levels, + .arch = _arch }; }; + + Attributes const orig_attributes = curr_attributes(); + + Xml_node const config = _config.xml(); + bool const report_state = - _config.xml().has_sub_node("report") && - _config.xml().sub_node("report").attribute_value("state", false); + config.has_sub_node("report") && + config.sub_node("report").attribute_value("state", false); _state_reporter.conditional(report_state, _env, "state", "state"); + _prio_levels = Child::Prio_levels { config.attribute_value("prio_levels", 0U) }; + _arch = config.attribute_value("arch", Arch()); + + bool const config_affected_child = _children.apply_config(config); + bool const blueprint_affected_child = _children.apply_blueprint(_blueprint.xml()); + + bool const progress = (curr_attributes() != orig_attributes) + || config_affected_child + || blueprint_affected_child; + if (!progress) + return; + if (_state_reporter.constructed()) _state_reporter->generate([&] (Xml_generator xml) { xml.attribute("running", true); }); - Xml_node const config = _config.xml(); - - _children.apply_config(config); - _children.apply_blueprint(_blueprint.xml()); - - Child::Prio_levels const prio_levels { - config.attribute_value("prio_levels", 0U) }; - - /* determine CPU architecture of deployment */ - typedef String<16> Arch; - Arch const arch = config.attribute_value("arch", Arch()); - if (!arch.valid()) + if (!_arch.valid()) warning("config lacks 'arch' attribute"); /* generate init config containing all configured start nodes */ _init_config_reporter.generate([&] (Xml_generator &xml) { - - if (prio_levels.value) - xml.attribute("prio_levels", prio_levels.value); - - config.with_sub_node("static", - [&] (Xml_node static_config) { - static_config.with_raw_content([&] (char const *start, size_t length) { - xml.append(start, length); }); - }, - [&] () { warning("config lacks node"); }); - - config.with_optional_sub_node("report", [&] (Xml_node const &report) { - - auto copy_bool_attribute = [&] (char const* name) { - if (report.has_attribute(name)) { - xml.attribute(name, report.attribute_value(name, false)); - } - }; - - auto copy_buffer_size_attribute = [&] () { - char const *name { "buffer" }; - if (report.has_attribute(name)) { - xml.attribute(name, report.attribute_value(name, - Number_of_bytes(4096))); - } - }; - - size_t const delay_ms = report.attribute_value("delay_ms", 1000UL); - xml.node("report", [&] () { - xml.attribute("delay_ms", delay_ms); - - /* attributes according to repos/os/src/lib/sandbox/report.h */ - copy_bool_attribute("ids"); - copy_bool_attribute("requested"); - copy_bool_attribute("provided"); - copy_bool_attribute("session_args"); - copy_bool_attribute("child_ram"); - copy_bool_attribute("child_caps"); - copy_bool_attribute("init_ram"); - copy_bool_attribute("init_caps"); - - /* attribute according to repos/os/src/init/main.cc */ - copy_buffer_size_attribute(); - }); - }); - - config.with_optional_sub_node("heartbeat", [&] (Xml_node const &heartbeat) { - size_t const rate_ms = heartbeat.attribute_value("rate_ms", 2000UL); - xml.node("heartbeat", [&] () { - xml.attribute("rate_ms", rate_ms); - }); - }); - - config.with_sub_node("common_routes", - [&] (Xml_node node) { - Child::Depot_rom_server const parent { }; - _children.gen_start_nodes(xml, node, - prio_levels, Affinity::Space(1, 1), - parent, parent); - }, - [&] () { warning("config lacks node"); }); - }); + _gen_init_config(xml, config); }); /* update query for blueprints of all unconfigured start nodes */ - if (arch.valid()) { + if (_arch.valid()) { _query_reporter.generate([&] (Xml_generator &xml) { - xml.attribute("arch", arch); + xml.attribute("arch", _arch); _children.gen_queries(xml); }); } @@ -160,12 +132,74 @@ struct Depot_deploy::Main } } + void _gen_init_config(Xml_generator &xml, Xml_node const &config) const + { + if (_prio_levels.value) + xml.attribute("prio_levels", _prio_levels.value); + + config.with_sub_node("static", + [&] (Xml_node static_config) { + static_config.with_raw_content([&] (char const *start, size_t length) { + xml.append(start, length); }); + }, + [&] () { warning("config lacks node"); }); + + config.with_optional_sub_node("report", [&] (Xml_node const &report) { + + auto copy_bool_attribute = [&] (auto const name) + { + if (report.has_attribute(name)) + xml.attribute(name, report.attribute_value(name, false)); + }; + + auto copy_buffer_size_attribute = [&] () + { + auto const name = "buffer"; + if (report.has_attribute(name)) + xml.attribute(name, report.attribute_value(name, Number_of_bytes(4096))); + }; + + size_t const delay_ms = report.attribute_value("delay_ms", 1000UL); + xml.node("report", [&] () { + xml.attribute("delay_ms", delay_ms); + + /* attributes according to repos/os/src/lib/sandbox/report.h */ + copy_bool_attribute("ids"); + copy_bool_attribute("requested"); + copy_bool_attribute("provided"); + copy_bool_attribute("session_args"); + copy_bool_attribute("child_ram"); + copy_bool_attribute("child_caps"); + copy_bool_attribute("init_ram"); + copy_bool_attribute("init_caps"); + + /* attribute according to repos/os/src/init/main.cc */ + copy_buffer_size_attribute(); + }); + }); + + config.with_optional_sub_node("heartbeat", [&] (Xml_node const &heartbeat) { + size_t const rate_ms = heartbeat.attribute_value("rate_ms", 2000UL); + xml.node("heartbeat", [&] () { + xml.attribute("rate_ms", rate_ms); + }); + }); + + config.with_sub_node("common_routes", + [&] (Xml_node node) { + Child::Depot_rom_server const parent { }; + _children.gen_start_nodes(xml, node, + _prio_levels, Affinity::Space(1, 1), + parent, parent); + }, + [&] () { warning("config lacks node"); }); + } + Main(Env &env) : _env(env) { _config .sigh(_config_handler); _blueprint.sigh(_config_handler); - _handle_config(); } }; diff --git a/repos/gems/src/app/sculpt_manager/deploy.cc b/repos/gems/src/app/sculpt_manager/deploy.cc index 473d5ba1b8..fe627f6c7b 100644 --- a/repos/gems/src/app/sculpt_manager/deploy.cc +++ b/repos/gems/src/app/sculpt_manager/deploy.cc @@ -88,63 +88,91 @@ void Sculpt::Deploy::handle_deploy() Xml_node const managed_deploy = _managed_deploy_rom.xml(); /* determine CPU architecture of deployment */ + Arch const orig_arch = _arch; _arch = managed_deploy.attribute_value("arch", Arch()); if ((managed_deploy.type() != "empty") && !_arch.valid()) warning("managed deploy config lacks 'arch' attribute"); - try { _children.apply_config(managed_deploy); } - catch (...) { - error("spurious exception during deploy update (apply_config)"); } + bool const arch_changed = (orig_arch != _arch); - /* - * Apply launchers - */ - Xml_node const launcher_listing = _launcher_listing_rom.xml(); - launcher_listing.for_each_sub_node("dir", [&] (Xml_node dir) { + auto apply_config = [&] + { + try { return _children.apply_config(managed_deploy); } + catch (...) { + error("spurious exception during deploy update (apply_config)"); } + return false; + }; - typedef String<20> Path; - Path const path = dir.attribute_value("path", Path()); + bool const config_affected_child = apply_config(); - if (path != "/launcher") - return; + auto apply_launchers = [&] + { + bool any_child_affected = false; - dir.for_each_sub_node("file", [&] (Xml_node file) { + Xml_node const launcher_listing = _launcher_listing_rom.xml(); + launcher_listing.for_each_sub_node("dir", [&] (Xml_node dir) { - if (file.attribute_value("xml", false) == false) + typedef String<20> Path; + Path const path = dir.attribute_value("path", Path()); + + if (path != "/launcher") return; - typedef Depot_deploy::Child::Launcher_name Name; - Name const name = file.attribute_value("name", Name()); + dir.for_each_sub_node("file", [&] (Xml_node file) { - file.for_each_sub_node("launcher", [&] (Xml_node launcher) { - _children.apply_launcher(name, launcher); }); + if (file.attribute_value("xml", false) == false) + return; + + typedef Depot_deploy::Child::Launcher_name Name; + Name const name = file.attribute_value("name", Name()); + + file.for_each_sub_node("launcher", [&] (Xml_node launcher) { + if (_children.apply_launcher(name, launcher)) + any_child_affected = true; }); + }); }); - }); + return any_child_affected; + }; - try { - Xml_node const blueprint = _blueprint_rom.xml(); + bool const launcher_affected_child = apply_launchers(); - /* apply blueprint, except when stale */ - typedef String<32> Version; - Version const version = blueprint.attribute_value("version", Version()); - if (version == Version(_depot_query.depot_query_version().value)) - _children.apply_blueprint(_blueprint_rom.xml()); + auto apply_blueprint = [&] + { + try { + Xml_node const blueprint = _blueprint_rom.xml(); + + /* apply blueprint, except when stale */ + typedef String<32> Version; + Version const version = blueprint.attribute_value("version", Version()); + if (version == Version(_depot_query.depot_query_version().value)) + return _children.apply_blueprint(_blueprint_rom.xml()); + } + catch (...) { + error("spurious exception during deploy update (apply_blueprint)"); } + return false; + }; + + bool const blueprint_affected_child = apply_blueprint(); + + bool const progress = arch_changed + || config_affected_child + || launcher_affected_child + || blueprint_affected_child; + if (progress) { + + /* update query for blueprints of all unconfigured start nodes */ + if (!_download_queue.any_active_download()) + _depot_query.trigger_depot_query(); + + /* feed missing packages to installation queue */ + update_installation(); + + /* apply runtime condition checks */ + update_child_conditions(); + + _dialog_generator.generate_dialog(); + _runtime_config_generator.generate_runtime_config(); } - catch (...) { - error("spurious exception during deploy update (apply_blueprint)"); } - - /* update query for blueprints of all unconfigured start nodes */ - if (_children.any_blueprint_needed() && !_download_queue.any_active_download()) - _depot_query.trigger_depot_query(); - - /* feed missing packages to installation queue */ - update_installation(); - - /* apply runtime condition checks */ - update_child_conditions(); - - _dialog_generator.generate_dialog(); - _runtime_config_generator.generate_runtime_config(); }