From 0e6d08495733ed268e1624ca6450a0f4ecccac33 Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Sat, 22 Apr 2017 01:30:16 +0300 Subject: Add support for forced rebuild --- mod/buildfile | 1 + mod/mod-build-force | 45 +++++++ mod/mod-build-force.cxx | 189 ++++++++++++++++++++++++++++++ mod/mod-build-result.cxx | 72 +++++++++--- mod/mod-build-task.cxx | 278 +++++++++++++++++++++++++++++++++----------- mod/mod-repository-root | 2 + mod/mod-repository-root.cxx | 23 +++- mod/options.cli | 55 ++++++++- 8 files changed, 572 insertions(+), 93 deletions(-) create mode 100644 mod/mod-build-force create mode 100644 mod/mod-build-force.cxx (limited to 'mod') diff --git a/mod/buildfile b/mod/buildfile index 2926a1b..771b52b 100644 --- a/mod/buildfile +++ b/mod/buildfile @@ -23,6 +23,7 @@ mod{brep}: \ {hxx cxx}{ database } \ {hxx cxx}{ database-module } \ {hxx cxx}{ diagnostics } \ + {hxx cxx}{ mod-build-force } \ {hxx cxx}{ mod-build-log } \ {hxx cxx}{ mod-build-result } \ {hxx cxx}{ mod-build-task } \ diff --git a/mod/mod-build-force b/mod/mod-build-force new file mode 100644 index 0000000..3fa723d --- /dev/null +++ b/mod/mod-build-force @@ -0,0 +1,45 @@ +// file : mod/mod-build-force -*- C++ -*- +// copyright : Copyright (c) 2014-2017 Code Synthesis Ltd +// license : MIT; see accompanying LICENSE file + +#ifndef MOD_MOD_BUILD_FORCE +#define MOD_MOD_BUILD_FORCE + +#include +#include + +#include +#include + +namespace brep +{ + class build_force: public database_module + { + public: + build_force () = default; + + // Create a shallow copy (handling instance) if initialized and a deep + // copy (context exemplar) otherwise. + // + explicit + build_force (const build_force&); + + virtual bool + handle (request&, response&); + + virtual const cli::options& + cli_options () const + { + return options::build_force::description (); + } + + private: + virtual void + init (cli::scanner&); + + private: + shared_ptr options_; + }; +} + +#endif // MOD_MOD_BUILD_FORCE diff --git a/mod/mod-build-force.cxx b/mod/mod-build-force.cxx new file mode 100644 index 0000000..1db25d1 --- /dev/null +++ b/mod/mod-build-force.cxx @@ -0,0 +1,189 @@ +// file : mod/mod-build-force.cxx -*- C++ -*- +// copyright : Copyright (c) 2014-2017 Code Synthesis Ltd +// license : MIT; see accompanying LICENSE file + +#include + +#include // replace() + +#include +#include + +#include + +#include +#include +#include +#include + +#include + +using namespace std; +using namespace bbot; +using namespace brep::cli; +using namespace odb::core; + +// While currently the user-defined copy constructor is not required (we don't +// need to deep copy nullptr's), it is a good idea to keep the placeholder +// ready for less trivial cases. +// +brep::build_force:: +build_force (const build_force& r) + : database_module (r), + options_ (r.initialized_ ? r.options_ : nullptr) +{ +} + +void brep::build_force:: +init (scanner& s) +{ + MODULE_DIAG; + + options_ = make_shared ( + s, unknown_mode::fail, unknown_mode::fail); + + database_module::init (static_cast (*options_), + options_->package_db_retry ()); + + if (options_->build_config_specified ()) + database_module::init (static_cast (*options_), + static_cast (*options_), + options_->build_db_retry ()); +} + +bool brep::build_force:: +handle (request& rq, response& rs) +{ + using brep::version; // Not to confuse with module::version. + + MODULE_DIAG; + + if (build_db_ == nullptr) + throw invalid_request (501, "not implemented"); + + params::build_force params; + + try + { + name_value_scanner s (rq.parameters ()); + params = params::build_force (s, unknown_mode::fail, unknown_mode::fail); + } + catch (const cli::exception& e) + { + throw invalid_request (400, e.what ()); + } + + const string& reason (params.reason ()); + + if (reason.empty ()) + throw invalid_request (400, "missing rebuild reason"); + + build_id id; + version version; // Keep for logging. + + try + { + string& p (params.package ()); + + if (p.empty ()) + throw invalid_argument ("empty package name"); + + // We accept the non-url-encoded version representation. Note that the + // parameter is already url-decoded by the web server, so we just restore + // the space character (that is otherwise forbidden in version + // representation) to the plus character. + // + string& v (params.version ()); + replace (v.begin (), v.end (), ' ', '+'); + + // Intercept exception handling to add the parsing error attribution. + // + try + { + version = brep::version (v); + } + catch (const invalid_argument& e) + { + throw invalid_argument ( + string ("invalid package version: ") + e.what ()); + } + + string& c (params.configuration ()); + + if (c.empty ()) + throw invalid_argument ("no configuration name"); + + id = build_id (package_id (move (p), version), move (c)); + } + catch (const invalid_argument& e) + { + throw invalid_request (400, e.what ()); + } + + // If the package build configuration expired (no such configuration, + // package, etc), then we respond with the 404 HTTP code (not found but may + // be available in the future). + // + auto config_expired = [] (const string& d) + { + throw invalid_request (404, "package build configuration expired: " + d); + }; + + // Make sure the build configuration still exists. + // + auto i ( + find_if ( + build_conf_->begin (), build_conf_->end (), + [&id] (const build_config& c) {return c.name == id.configuration;})); + + if (i == build_conf_->end ()) + config_expired ("no configuration"); + + // Make sure the package still exists. + // + { + transaction t (package_db_->begin ()); + shared_ptr p (package_db_->find (id.package)); + t.commit (); + + if (p == nullptr) + config_expired ("no package"); + } + + // Load the package build configuration (if present), set the force flag and + // update the object's persistent state. + // + { + transaction t (build_db_->begin ()); + shared_ptr b (build_db_->find (id)); + + if (b == nullptr) + config_expired ("no package configuration"); + + // Respond with 409 (conflict) if the package configuration is in + // inappropriate state for being rebuilt. + // + else if (b->state != build_state::tested) + throw invalid_request (409, "state is " + to_string (b->state)); + + if (!b->forced) + { + b->forced = true; + build_db_->update (b); + + l1 ([&]{trace << "force rebuild for " + << id.package.name << '/' << version << ' ' + << id.configuration << ": " << reason;}); + } + + t.commit (); + } + + // We have all the data, so don't buffer the response content. + // + ostream& os (rs.content (200, "text/plain;charset=utf-8", false)); + os << "Rebuilding in " << options_->build_forced_rebuild_timeout () + << " seconds."; + + return true; +} diff --git a/mod/mod-build-result.cxx b/mod/mod-build-result.cxx index fb6edb5..be0075b 100644 --- a/mod/mod-build-result.cxx +++ b/mod/mod-build-result.cxx @@ -17,6 +17,7 @@ #include #include +#include #include #include @@ -28,6 +29,7 @@ using namespace std; using namespace butl; using namespace bbot; +using namespace web; using namespace brep::cli; using namespace odb::core; @@ -193,6 +195,9 @@ handle (request& rq, response&) // Load and update the package build configuration (if present). // shared_ptr b; + optional prev_status; + bool notify (false); + { transaction t (build_db_->begin ()); b = build_db_->find (id); @@ -200,42 +205,55 @@ handle (request& rq, response&) if (b == nullptr) warn_expired ("no package configuration"); else if (b->state != build_state::testing) - { warn_expired ("package configuration state is " + to_string (b->state)); - b = nullptr; - } else { - b->state = build_state::tested; - b->timestamp = timestamp::clock::now (); + // Don's send email for the success-to-success status change, unless the + // build was forced. + // + notify = !(rqm.result.status == result_status::success && + b->status && *b->status == rqm.result.status && !b->forced); + + prev_status = move (b->status); - assert (!b->status); + b->state = build_state::tested; b->status = rqm.result.status; + b->forced = false; - // Need to manually load the lazy-load section prior to changing its - // members and updating the object's persistent state. + // Mark the section as loaded, so results are updated. // - // @@ TODO: ODB now allows marking a section as loaded so can optimize - // this. - // - build_db_->load (*b, b->results_section); + b->results_section.load (); b->results = move (rqm.result.results); + b->timestamp = timestamp::clock::now (); + build_db_->update (b); } t.commit (); } - if (b == nullptr) - return true; // Warning is already logged. + if (!notify) + return true; + + assert (b != nullptr); // Send email to the package owner. // try { string subj (b->package_name + '/' + b->package_version.string () + ' ' + - b->configuration + " build: " + to_string (*b->status)); + b->configuration); + + if (!prev_status) + subj += " build: " + to_string (*b->status); + else + { + subj += " rebuild: " + to_string (*b->status); + + if (*prev_status != *b->status) + subj += " after " + to_string (*prev_status); + } // If the package email address is not specified, then it is assumed to be // the same as the project email address. @@ -264,11 +282,27 @@ handle (request& rq, response&) sm.out << "No operations results available." << endl; else { + string url (options_->host () + options_->root ().string ()); + string pkg (mime_url_encode (b->package_name)); + string cfg (mime_url_encode (b->configuration)); + + // Note that '+' is the only package version character that potentially + // needs to be url-encoded, and only in the query part of the URL. + // However, we print the package version either as part of URL path or + // as the build-force URL query part (where it is not encoded by + // design). + // + const version& ver (b->package_version); + for (const auto& r: b->results) - sm.out << r.operation << ": " << r.status << ", " - << options_->host () << options_->root ().representation () - << b->package_name << '/' << b->package_version << "/log/" - << b->configuration << '/' << r.operation << endl; + sm.out << r.operation << ": " << r.status << ", " << url << '/' << pkg + << '/' << ver << "/log/" << cfg << '/' << r.operation << endl; + + sm.out << endl + << "force rebuild: " << url << "?build-force&p=" << pkg + << "&v=" << ver << "&c=" << cfg << "&reason=" << endl << endl + << "Note: enter the rebuild reason in the above URL (" + << "using '+' instead of space characters)." << endl; } sm.out.close (); diff --git a/mod/mod-build-task.cxx b/mod/mod-build-task.cxx index 1cb5386..1466077 100644 --- a/mod/mod-build-task.cxx +++ b/mod/mod-build-task.cxx @@ -127,22 +127,80 @@ handle (request& rq, response& rs) // Go through packages until we find one that has no build configuration // present in the database, or has the untested one, or in the testing state - // but expired, or the one, which build failed abnormally and expired. If - // such a package configuration is found then put it into the testing state, - // set the current timestamp and respond with the task for building this - // package configuration. + // but expired (collectively called unbuilt). If such a package + // configuration is found then put it into the testing state, set the + // current timestamp and respond with the task for building this package + // configuration. + // + // While trying to find a non-built package configuration we will also + // collect the list of the tested package configurations which it's time to + // rebuilt. So if no unbuilt package is found, we will pickup one to + // rebuild. The rebuild preference is given in the following order: the + // greater force flag, the greater overall status, the lower timestamp. // if (!cfg_machines.empty ()) { + vector> rebuilds; + + // Create the task response manifest. The package must have the internal + // repository loaded. + // + auto task = [this] (shared_ptr&& b, + shared_ptr&& p, + const config_machine& cm) -> task_response_manifest + { + string session (b->package_name + '/' + b->package_version.string () + + '/' + b->configuration); + + string result_url (options_->host () + options_->root ().string () + + "?build-result"); + + lazy_shared_ptr r (p->internal_repository); + + strings fp; + if (r->certificate) + fp.emplace_back (move (r->certificate->fingerprint)); + + task_manifest task (move (b->package_name), + move (b->package_version), + move (r->location), + move (fp), + cm.machine->name, + cm.config->target, + cm.config->vars); + + // @@ We don't support challenge at the moment. + // + return task_response_manifest (move (session), + nullopt, + move (result_url), + move (task)); + }; + // Calculate the expiration time for package configurations being in the - // testing state or those, which build failed abnormally. + // testing (build expiration) or the tested (rebuild expiration) state. // - timestamp expiration (timestamp::clock::now () - - chrono::seconds (options_->build_result_timeout ())); + timestamp now (timestamp::clock::now ()); + + auto expiration = [&now] (size_t timeout) -> timestamp + { + return now - chrono::seconds (timeout); + }; + + auto expiration_ns = [&expiration] (size_t timeout) -> uint64_t + { + return chrono::duration_cast ( + expiration (timeout).time_since_epoch ()).count (); + }; - uint64_t expiration_ns ( - std::chrono::duration_cast ( - expiration.time_since_epoch ()).count ()); + uint64_t build_expiration_ns ( + expiration_ns (options_->build_result_timeout ())); + + timestamp forced_rebuild_expiration ( + expiration (options_->build_forced_rebuild_timeout ())); + + timestamp normal_rebuild_expiration ( + expiration (options_->build_normal_rebuild_timeout ())); // Prepare the package version prepared query. // @@ -188,10 +246,9 @@ handle (request& rq, response& rs) // configurations that have already been acted upon (initially empty). // // This is why we query the database for package configurations that - // should not be built (in the tested state with the build terminated - // normally or not expired, or in the testing state and not expired). - // Having such a list we will select the first build configuration that is - // not in the list (if available) for the response. + // should not be built (in the tested state, or in the testing state and + // not expired). Having such a list we will select the first build + // configuration that is not in the list (if available) for the response. // using bld_query = query; using prep_bld_query = prepared_query; @@ -212,12 +269,9 @@ handle (request& rq, response& rs) bld_query::id.configuration.in_range (cfg_names.begin (), cfg_names.end ()) && - ((bld_query::state == "tested" && - ((bld_query::status != "abort" && bld_query::status != "abnormal") || - bld_query::timestamp > expiration_ns)) || - + (bld_query::state == "tested" || (bld_query::state == "testing" && - bld_query::timestamp > expiration_ns))); + bld_query::timestamp > build_expiration_ns))); connection_ptr bld_conn (build_db_->connection ()); @@ -262,25 +316,33 @@ handle (request& rq, response& rs) // configurations that remained can be built. We will take the first // one, if present. // + // Also save the tested package configurations for which it's time + // to be rebuilt. + // config_machines configs (cfg_machines); // Make a copy for this pkg. + auto pkg_builds (bld_prep_query.execute ()); - for (const auto& pc: bld_prep_query.execute ()) + for (auto i (pkg_builds.begin ()); i != pkg_builds.end (); ++i) { - auto i (configs.find (pc.id.configuration.c_str ())); + auto j (configs.find (i->id.configuration.c_str ())); // Outdated configurations are already excluded with the database // query. // - assert (i != configs.end ()); - configs.erase (i); + assert (j != configs.end ()); + configs.erase (j); + + if (i->state == build_state::tested && + i->timestamp <= (i->forced + ? forced_rebuild_expiration + : normal_rebuild_expiration)) + rebuilds.emplace_back (i.load ()); } if (!configs.empty ()) { config_machine& cm (configs.begin ()->second); - const build_config& cfg (*cm.config); - - build_id bid (move (id), cfg.name); + build_id bid (move (id), cm.config->name); shared_ptr b (build_db_->find (bid)); // If build configuration doesn't exist then create the new one @@ -297,27 +359,30 @@ handle (request& rq, response& rs) } else { - // If the package configuration is in the tested state, then we - // need to cleanup the status and results prior to the update. - // Otherwise the status is already absent and there are no - // results. + // The package configuration can be in the testing or untested + // state, so the forced flag is false and the status is absent, + // unless in the testing state (in which case they may or may not + // be set/exist), and there are no results. + // + // Note that in the testing state the status can be present if + // the rebuild task have been issued. In both cases we keep the + // status intact to be able to compare it with the final one in + // the result request handling in order to decide if to send the + // notification email. The same is true for the forced flag. We + // just assert that if the force flag is set, then the status + // exists. // - // Load the section to make sure results are updated for the - // tested state, otherwise assert there are no results. + // Load the section to assert the above statement. // build_db_->load (*b, b->results_section); - if (b->state == build_state::tested) - { - assert (b->status); - b->status = nullopt; + assert (b->state != build_state::tested && + + ((!b->forced && !b->status) || + (b->state == build_state::testing && + (!b->forced || b->status))) && - b->results.clear (); - } - else - { - assert (!b->status && b->results.empty ()); - } + b->results.empty ()); b->state = build_state::testing; b->timestamp = timestamp::clock::now (); @@ -327,41 +392,21 @@ handle (request& rq, response& rs) // Finally, prepare the task response manifest. // - tsm.session = b->package_name + '/' + - b->package_version.string () + '/' + b->configuration; - - // @@ We don't support challenge at the moment, so leave it absent. - // - - tsm.result_url = options_->host () + options_->root ().string () + - "?build-result"; - // Switch to the package database transaction to load the package. // transaction::current (pt); shared_ptr p (package_db_->load (b->id.package)); - shared_ptr r (p->internal_repository.load ()); + p->internal_repository.load (); // Switch back to the build database transaction. // transaction::current (bt); - strings fp; - if (r->certificate) - fp.emplace_back (move (r->certificate->fingerprint)); - - tsm.task = task_manifest ( - move (b->package_name), - move (b->package_version), - move (r->location), - move (fp), - move (cm.machine->name), - cfg.target, - cfg.vars); + tsm = task (move (b), move (p), cm); } - // If task response manifest is filled, then can bail out from the + // If the task response manifest is prepared, then bail out from the // package loop, commit transactions and respond. // if (!tsm.session.empty ()) @@ -374,6 +419,109 @@ handle (request& rq, response& rs) transaction::current (pt); // Switch to the package database transaction. pt.commit (); } + + // If we don't have an unbuilt package, then let's see if we have a + // package to rebuild. + // + if (tsm.session.empty () && !rebuilds.empty ()) + { + // Sort the package configuration rebuild list with the following sort + // priority: + // + // 1: forced flag + // 2: overall status + // 3: timestamp (less is preferred) + // + auto cmp = [] (const shared_ptr& x, + const shared_ptr& y) -> bool + { + if (x->forced != y->forced) + return x->forced > y->forced; // Forced goes first. + + assert (x->status && y->status); // Both tested. + + if (x->status != y->status) + return x->status > y->status; // Larger status goes first. + + return x->timestamp < y->timestamp; // Older goes first. + }; + + sort (rebuilds.begin (), rebuilds.end (), cmp); + + // Pick the first package configuration from the ordered list. + // + // Note that the configurations may not match the required criteria + // anymore (as we have committed the database transactions that were + // used to collect this data) so we recheck. If we find one that matches + // then put it into the testing state, refresh the timestamp and + // update. Note that we don't amend the status and the force flag to + // have them available in the result request handling (see above). + // + for (auto& b: rebuilds) + { + try + { + transaction bt (build_db_->begin ()); + + b = build_db_->find (b->id); + + if (b != nullptr && b->state == build_state::tested && + b->timestamp <= (b->forced + ? forced_rebuild_expiration + : normal_rebuild_expiration)) + { + // Load the package (if still present). + // + transaction pt (package_db_->begin (), false); + transaction::current (pt); + + shared_ptr p (package_db_->find (b->id.package)); + + if (p != nullptr) + p->internal_repository.load (); + + // Commit the package database transaction and switch back to the + // build database transaction. + // + pt.commit (); + transaction::current (bt); + + if (p != nullptr) + { + assert (b->status); + + b->state = build_state::testing; + + // Mark the section as loaded, so results are updated. + // + b->results_section.load (); + b->results.clear (); + + b->timestamp = timestamp::clock::now (); + + build_db_->update (b); + + auto i (cfg_machines.find (b->id.configuration.c_str ())); + + // Only actual package configurations are loaded (see above). + // + assert (i != cfg_machines.end ()); + + tsm = task (move (b), move (p), i->second); + } + } + + bt.commit (); + } + catch (const odb::deadlock&) {} // Just try with the next rebuild. + + // If the task response manifest is prepared, then bail out from the + // package configuration rebuilds loop and respond. + // + if (!tsm.session.empty ()) + break; + } + } } // @@ Probably it would be a good idea to also send some cache control diff --git a/mod/mod-repository-root b/mod/mod-repository-root index 6a40e10..b347cd3 100644 --- a/mod/mod-repository-root +++ b/mod/mod-repository-root @@ -19,6 +19,7 @@ namespace brep class repository_details; class build_task; class build_result; + class build_force; class build_log; class repository_root: public module @@ -60,6 +61,7 @@ namespace brep shared_ptr repository_details_; shared_ptr build_task_; shared_ptr build_result_; + shared_ptr build_force_; shared_ptr build_log_; shared_ptr options_; diff --git a/mod/mod-repository-root.cxx b/mod/mod-repository-root.cxx index e156308..d8d7c64 100644 --- a/mod/mod-repository-root.cxx +++ b/mod/mod-repository-root.cxx @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -61,6 +62,7 @@ namespace brep repository_details_ (make_shared ()), build_task_ (make_shared ()), build_result_ (make_shared ()), + build_force_ (make_shared ()), build_log_ (make_shared ()) { } @@ -97,6 +99,10 @@ namespace brep r.initialized_ ? r.build_result_ : make_shared (*r.build_result_)), + build_force_ ( + r.initialized_ + ? r.build_force_ + : make_shared (*r.build_force_)), build_log_ ( r.initialized_ ? r.build_log_ @@ -121,6 +127,7 @@ namespace brep append (r, repository_details_->options ()); append (r, build_task_->options ()); append (r, build_result_->options ()); + append (r, build_force_->options ()); append (r, build_log_->options ()); return r; } @@ -161,6 +168,7 @@ namespace brep sub_init (*repository_details_, "repository_details"); sub_init (*build_task_, "build_task"); sub_init (*build_result_, "build_result"); + sub_init (*build_force_, "build_force"); sub_init (*build_log_, "build_log"); // Parse own configuration options. @@ -235,10 +243,10 @@ namespace brep // if (lpath.empty ()) { - // Dispatch request handling to the repository_details, the build_task, - // the build_result or the package_search module depending on the - // function name passed as a first HTTP request parameter. The parameter - // should have no value specified. Example: cppget.org/?about + // Dispatch request handling to the repository_details, the + // package_search or the one of build_* modules depending on the function + // name passed as a first HTTP request parameter. The parameter should + // have no value specified. Example: cppget.org/?about // const name_values& params (rq.parameters ()); if (!params.empty () && !params.front ().value) @@ -273,6 +281,13 @@ namespace brep return handle (rp, "build_result"); } + else if (fn == "build-force") + { + if (handler_ == nullptr) + handler_.reset (new build_force (*build_force_)); + + return handle (rp, "build_force"); + } else throw invalid_request (400, "unknown function"); } diff --git a/mod/options.cli b/mod/options.cli index 4dbcd89..0f96e9c 100644 --- a/mod/options.cli +++ b/mod/options.cli @@ -116,6 +116,20 @@ namespace brep functionality will be disabled. If specified, then the build database must be configured (see \cb{build-db-*})." } + + size_t build-forced-rebuild-timeout = 600 + { + "", + "Time to wait before considering a package for a forced rebuild. Must + be specified in seconds. Default is 10 minutes." + } + + size_t build-normal-rebuild-timeout = 86400 + { + "", + "Time to wait before considering a package for a normal rebuild. Must + be specified in seconds. Default is 24 hours." + } }; class build_db @@ -248,10 +262,6 @@ namespace brep { }; - class repository_root: module - { - }; - class build_task: build, package_db, build_db, module { size_t build-task-request-max-size = 102400 @@ -265,7 +275,7 @@ namespace brep size_t build-result-timeout = 10800 { - "", + "", "Time to wait before considering the expected task result lost. Must be specified in seconds. The default is 3 hours." } @@ -286,6 +296,14 @@ namespace brep class build_log: build, package_db, build_db, module { }; + + class build_force: build, package_db, build_db, module + { + }; + + class repository_root: module + { + }; } // Web module HTTP request parameters. @@ -351,5 +369,32 @@ namespace brep // No parameters so far. // }; + + // All parameters are non-optional. + // + class build_force + { + // Package name. + // + string package | p; + + // Package version. May not be url-encoded, in which case the plus + // character is considered literally (rather than as the encoded space + // character). In other words, after url-decoding the space character is + // treated the same way as the plus character. + // + // @@ Make it of the version type? Maybe after it get moved to bpkg/types + // or at least the second use case appear. + // + string version | v; + + // Package build configuration. + // + string configuration | c; + + // Package rebuild reason. Must not be empty. + // + string reason; + }; } } -- cgit v1.1