From 6e90b57a442424876b1325b9209f79c8a885a479 Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Tue, 4 Jul 2017 11:27:47 +0300 Subject: Make use of foreign package objects in build-related functionality --- mod/mod-build-force.cxx | 24 +-- mod/mod-build-log.cxx | 27 +--- mod/mod-build-result.cxx | 15 +- mod/mod-build-task.cxx | 302 ++++++++++++++++-------------------- mod/mod-builds.cxx | 28 ++-- mod/mod-package-version-details.cxx | 3 +- mod/options.cli | 2 +- 7 files changed, 169 insertions(+), 232 deletions(-) (limited to 'mod') diff --git a/mod/mod-build-force.cxx b/mod/mod-build-force.cxx index 325a1c7..a1d424c 100644 --- a/mod/mod-build-force.cxx +++ b/mod/mod-build-force.cxx @@ -13,8 +13,6 @@ #include #include -#include -#include #include @@ -42,9 +40,6 @@ init (scanner& s) 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_), @@ -145,27 +140,18 @@ handle (request& rq, response& rs) build_conf_map_->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"); + package_build pb; + if (!build_db_->query_one ( + query::build::id == id, pb)) + config_expired ("no package build"); + shared_ptr b (pb.build); force_state force (b->state == build_state::built ? force_state::forced : force_state::forcing); diff --git a/mod/mod-build-log.cxx b/mod/mod-build-log.cxx index 281eec6..f9fb0e5 100644 --- a/mod/mod-build-log.cxx +++ b/mod/mod-build-log.cxx @@ -15,8 +15,6 @@ #include #include -#include -#include #include @@ -44,9 +42,6 @@ init (scanner& s) 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_), @@ -174,27 +169,19 @@ handle (request& rq, response& rs) build_conf_map_->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). // shared_ptr b; { transaction t (build_db_->begin ()); - b = build_db_->find (id); - if (b == nullptr) - config_expired ("no package configuration"); - else if (b->state != build_state::built) + package_build pb; + if (!build_db_->query_one ( + query::build::id == id, pb)) + config_expired ("no package build"); + + b = pb.build; + if (b->state != build_state::built) config_expired ("state is " + to_string (b->state)); else build_db_->load (*b, b->results_section); diff --git a/mod/mod-build-result.cxx b/mod/mod-build-result.cxx index 042114d..1f5eb69 100644 --- a/mod/mod-build-result.cxx +++ b/mod/mod-build-result.cxx @@ -215,6 +215,12 @@ handle (request& rq, response&) // Load the built package (if present). // + // The only way not to deal with 2 databases simultaneously is to pull + // another bunch of the package fields into the build_package foreign + // object, which is a pain (see build_package.hxx for details). Doesn't seem + // worth it here: email members are really secondary and we don't need to + // switch transactions back and forth. + // shared_ptr p; { transaction t (package_db_->begin ()); @@ -242,11 +248,12 @@ handle (request& rq, response&) { transaction t (build_db_->begin ()); - b = build_db_->find (id); - if (b == nullptr) - warn_expired ("no package configuration"); - else if (b->state != build_state::building) + package_build pb; + if (!build_db_->query_one ( + query::build::id == id, pb)) + warn_expired ("no package build"); + else if ((b = pb.build)->state != build_state::building) warn_expired ("package configuration state is " + to_string (b->state)); else if (b->timestamp != session_timestamp) warn_expired ("non-matching timestamp"); diff --git a/mod/mod-build-task.cxx b/mod/mod-build-task.cxx index 86acbf6..1a7b272 100644 --- a/mod/mod-build-task.cxx +++ b/mod/mod-build-task.cxx @@ -27,8 +27,8 @@ #include #include -#include -#include +#include +#include #include @@ -57,9 +57,6 @@ init (scanner& s) 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_), @@ -165,7 +162,7 @@ handle (request& rq, response& rs) // // While trying to find a non-built package configuration we will also // collect the list of the built package configurations which it's time to - // rebuilt. So if no unbuilt package is found, we will pickup one to + // rebuild. 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 state, the greater overall status, the lower timestamp. // @@ -177,7 +174,7 @@ handle (request& rq, response& rs) // repository loaded. // auto task = [this] (shared_ptr&& b, - shared_ptr&& p, + shared_ptr&& p, const config_machine& cm) -> task_response_manifest { uint64_t ts ( @@ -192,11 +189,11 @@ handle (request& rq, response& rs) string result_url (options_->host () + options_->root ().string () + "?build-result"); - lazy_shared_ptr r (p->internal_repository); + lazy_shared_ptr r (p->internal_repository); strings fp; - if (r->certificate) - fp.emplace_back (move (r->certificate->fingerprint)); + if (r->certificate_fingerprint) + fp.emplace_back (move (*r->certificate_fingerprint)); task_manifest task (move (b->package_name), move (b->package_version), @@ -302,7 +299,7 @@ handle (request& rq, response& rs) // brep::version toolchain_version (tqm.toolchain_version.string ()); - // Prepare the package version prepared query. + // Prepare the buildable package prepared query. // // Note that the number of packages can be large and so, in order not to // hold locks for too long, we will restrict the number of packages being @@ -314,41 +311,36 @@ handle (request& rq, response& rs) // harmful in that: updates are infrequent and missed packages will be // picked up on the next request. // - using pkg_query = query; - using prep_pkg_query = prepared_query; - - size_t offset (0); // See the package version query. + using pkg_query = query; + using prep_pkg_query = prepared_query; - // Skip external and stub packages. - // - pkg_query pq (pkg_query::package::internal_repository.is_not_null () && - compare_version_ne (pkg_query::package::id.version, - wildcard_version, - false)); + pkg_query pq (true); - // Filter by repositories display names (if requested). + // Filter by repositories canonical names (if requested). // const vector& rp (params.repository ()); if (!rp.empty ()) pq = pq && - pkg_query::repository::display_name.in_range (rp.begin (), rp.end ()); + pkg_query::build_repository::name.in_range (rp.begin (), rp.end ()); // Specify the portion. // + size_t offset (0); + pq += "ORDER BY" + - pkg_query::package::id.name + "," + - pkg_query::package::id.version.epoch + "," + - pkg_query::package::id.version.canonical_upstream + "," + - pkg_query::package::id.version.canonical_release + "," + - pkg_query::package::id.version.revision + + pkg_query::build_package::id.name + "," + + pkg_query::build_package::id.version.epoch + "," + + pkg_query::build_package::id.version.canonical_upstream + "," + + pkg_query::build_package::id.version.canonical_release + "," + + pkg_query::build_package::id.version.revision + "OFFSET" + pkg_query::_ref (offset) + "LIMIT 50"; - connection_ptr pkg_conn (package_db_->connection ()); + connection_ptr conn (build_db_->connection ()); prep_pkg_query pkg_prep_query ( - pkg_conn->prepare_query ( - "mod-build-task-package-version-query", pq)); + conn->prepare_query ( + "mod-build-task-package-query", pq)); // Prepare the build prepared query. // @@ -364,8 +356,7 @@ handle (request& rq, response& rs) using bld_query = query; using prep_bld_query = prepared_query; - package_id id; // See the build query. - + package_id id; const auto& qv (bld_query::id.package.version); bld_query bq ( @@ -390,169 +381,147 @@ handle (request& rq, response& rs) (bld_query::force != "forcing" && // Unforced or forced. bld_query::timestamp > normal_result_expiration_ns)))); - connection_ptr bld_conn (build_db_->connection ()); - prep_bld_query bld_prep_query ( - bld_conn->prepare_query ( - "mod-build-task-package-build-query", bq)); + conn->prepare_query ("mod-build-task-build-query", bq)); while (tsm.session.empty ()) { - // Start the package database transaction. - // - transaction pt (pkg_conn->begin ()); + transaction t (conn->begin ()); - // Query package versions. + // Query (and cache) buildable packages. // - auto package_versions (pkg_prep_query.execute ()); + auto packages (pkg_prep_query.execute ()); // Bail out if there is nothing left. // - if (package_versions.empty ()) + if (packages.empty ()) { - pt.commit (); + t.commit (); break; } - offset += package_versions.size (); + offset += packages.size (); - // Start the build database transaction. + // Iterate over packages until we find one that needs building. // + for (auto& bp: packages) { - transaction bt (bld_conn->begin (), false); - transaction::current (bt); + id = move (bp.id); - // Iterate over packages until we find one that needs building. + // Iterate through the package configurations and erase those that + // don't need building from the build configuration map. All those + // configurations that remained can be built. We will take the first + // one, if present. // - for (auto& pv: package_versions) + // Also save the built 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 (auto i (pkg_builds.begin ()); i != pkg_builds.end (); ++i) { - id = move (pv.id); + auto j (configs.find (i->id.configuration.c_str ())); - // Iterate through the package configurations and erase those that - // don't need building from the build configuration map. All those - // configurations that remained can be built. We will take the first - // one, if present. + // Outdated configurations are already excluded with the database + // query. // - // Also save the built 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 ()); + assert (j != configs.end ()); + configs.erase (j); - for (auto i (pkg_builds.begin ()); i != pkg_builds.end (); ++i) + if (i->state == build_state::built) { - auto j (configs.find (i->id.configuration.c_str ())); - - // Outdated configurations are already excluded with the database - // query. - // - assert (j != configs.end ()); - configs.erase (j); - - if (i->state == build_state::built) - { - assert (i->force != force_state::forcing); + assert (i->force != force_state::forcing); - if (i->timestamp <= (i->force == force_state::forced - ? forced_rebuild_expiration - : normal_rebuild_expiration)) - rebuilds.emplace_back (i.load ()); - } + if (i->timestamp <= (i->force == force_state::forced + ? forced_rebuild_expiration + : normal_rebuild_expiration)) + rebuilds.emplace_back (i.load ()); } + } - if (!configs.empty ()) + if (!configs.empty ()) + { + config_machine& cm (configs.begin ()->second); + machine_header_manifest& mh (*cm.machine); + build_id bid (move (id), cm.config->name, toolchain_version); + shared_ptr b (build_db_->find (bid)); + optional cl (challenge ()); + + // If build configuration doesn't exist then create the new one and + // persist. Otherwise put it into the building state, refresh the + // timestamp and update. + // + if (b == nullptr) { - config_machine& cm (configs.begin ()->second); - machine_header_manifest& mh (*cm.machine); - build_id bid (move (id), cm.config->name, toolchain_version); - shared_ptr b (build_db_->find (bid)); - optional cl (challenge ()); - - // If build configuration doesn't exist then create the new one - // and persist. Otherwise put it into the building state, refresh - // the timestamp and update. + b = make_shared (move (bid.package.name), + move (bp.version), + move (bid.configuration), + move (tqm.toolchain_name), + move (toolchain_version), + move (agent_fp), + move (cl), + mh.name, + move (mh.summary), + cm.config->target); + + build_db_->persist (b); + } + else + { + // The package configuration is in the building state, and there + // are no results. // - if (b == nullptr) - { - b = make_shared (move (bid.package.name), - move (pv.version), - move (bid.configuration), - move (tqm.toolchain_name), - move (toolchain_version), - move (agent_fp), - move (cl), - mh.name, - move (mh.summary), - cm.config->target); - - build_db_->persist (b); - } - else - { - // The package configuration is in the building state, and there - // are no results. - // - // Note that 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 (in the sense that we - // don't set the force state to unforced). - // - // Load the section to assert the above statement. - // - build_db_->load (*b, b->results_section); - - assert (b->state == build_state::building && - b->results.empty ()); - - b->state = build_state::building; - - // Switch the force state not to reissue the task after the - // forced rebuild timeout. Note that the result handler will - // still recognize that the rebuild was forced. - // - if (b->force == force_state::forcing) - b->force = force_state::forced; - - b->toolchain_name = move (tqm.toolchain_name); - b->agent_fingerprint = move (agent_fp); - b->agent_challenge = move (cl); - b->machine = mh.name; - b->machine_summary = move (mh.summary); - b->target = cm.config->target; - b->timestamp = timestamp::clock::now (); - - build_db_->update (b); - } - - // Finally, prepare the task response manifest. + // Note that 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 (in the sense that we don't set the + // force state to unforced). // - // Switch to the package database transaction to load the package. + // Load the section to assert the above statement. // - transaction::current (pt); + build_db_->load (*b, b->results_section); - shared_ptr p (package_db_->load (b->id.package)); - p->internal_repository.load (); + assert (b->state == build_state::building && b->results.empty ()); - // Switch back to the build database transaction. - // - transaction::current (bt); + b->state = build_state::building; - tsm = task (move (b), move (p), cm); + // Switch the force state not to reissue the task after the forced + // rebuild timeout. Note that the result handler will still + // recognize that the rebuild was forced. + // + if (b->force == force_state::forcing) + b->force = force_state::forced; + + b->toolchain_name = move (tqm.toolchain_name); + b->agent_fingerprint = move (agent_fp); + b->agent_challenge = move (cl); + b->machine = mh.name; + b->machine_summary = move (mh.summary); + b->target = cm.config->target; + b->timestamp = timestamp::clock::now (); + + build_db_->update (b); } - // If the task response manifest is prepared, then bail out from the - // package loop, commit transactions and respond. + // Finally, prepare the task response manifest. // - if (!tsm.session.empty ()) - break; + shared_ptr p ( + build_db_->load (b->id.package)); + + p->internal_repository.load (); + + tsm = task (move (b), move (p), cm); } - bt.commit (); // Commit the build database transaction. + // If the task response manifest is prepared, then bail out from the + // package loop, commit transactions and respond. + // + if (!tsm.session.empty ()) + break; } - transaction::current (pt); // Switch to the package database transaction. - pt.commit (); + t.commit (); } // If we don't have an unbuilt package, then let's see if we have a @@ -598,7 +567,7 @@ handle (request& rq, response& rs) { try { - transaction bt (build_db_->begin ()); + transaction t (build_db_->begin ()); b = build_db_->find (b->id); @@ -617,19 +586,8 @@ handle (request& rq, response& rs) // 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); + shared_ptr p ( + build_db_->find (b->id.package)); if (p != nullptr) { @@ -656,11 +614,13 @@ handle (request& rq, response& rs) build_db_->update (b); + p->internal_repository.load (); + tsm = task (move (b), move (p), cm); } } - bt.commit (); + t.commit (); } catch (const odb::deadlock&) {} // Just try with the next rebuild. diff --git a/mod/mod-builds.cxx b/mod/mod-builds.cxx index 979121f..439bd05 100644 --- a/mod/mod-builds.cxx +++ b/mod/mod-builds.cxx @@ -205,11 +205,9 @@ package_query (const brep::params::builds& params) { using namespace brep; using query = query; + using qp = typename query::build_package; - // Skip external and stub packages. - // - query q (query::internal_repository.is_not_null () && - compare_version_ne (query::id.version, wildcard_version, false)); + query q (true); // Note that there is no error reported if the filter parameters parsing // fails. Instead, it is considered that no packages match such a query. @@ -219,12 +217,12 @@ package_query (const brep::params::builds& params) // Package name. // if (!params.name ().empty ()) - q = q && query::id.name.like (transform (params.name ())); + q = q && qp::id.name.like (transform (params.name ())); // Package version. // if (!params.version ().empty () && params.version () != "*") - q = q && compare_version_eq (query::id.version, + q = q && compare_version_eq (qp::id.version, version (params.version ()), // May throw. true); } @@ -554,8 +552,8 @@ handle (request& rq, response& rs) // configurations and the number of existing package builds. // size_t nmax (config_toolchains.size () * - build_db_->query_value ( - package_query (params))); + build_db_->query_value ( + package_query (params))); size_t ncur = build_db_->query_value ( build_query (*build_conf_names_, bld_params)); @@ -592,10 +590,10 @@ handle (request& rq, response& rs) // the pager to just '' links, and pass the offset as a // URL query parameter. Alternatively, we can invent the page number cap. // - using pkg_query = query; - using prep_pkg_query = prepared_query; + using pkg_query = query; + using prep_pkg_query = prepared_query; - pkg_query pq (package_query (params)); + pkg_query pq (package_query (params)); // Specify the portion. Note that we will still be querying packages in // chunks, not to hold locks for too long. @@ -603,14 +601,14 @@ handle (request& rq, response& rs) size_t offset (0); pq += "ORDER BY" + - pkg_query::id.name + - order_by_version_desc (pkg_query::id.version, false) + + pkg_query::build_package::id.name + + order_by_version_desc (pkg_query::build_package::id.version, false) + "OFFSET" + pkg_query::_ref (offset) + "LIMIT 50"; connection_ptr conn (build_db_->connection ()); prep_pkg_query pkg_prep_query ( - conn->prepare_query ("mod-builds-package-query", pq)); + conn->prepare_query ("mod-builds-package-query", pq)); // Prepare the build prepared query. // @@ -655,7 +653,7 @@ handle (request& rq, response& rs) { transaction t (conn->begin ()); - // Query (and cache) build packages. + // Query (and cache) buildable packages. // auto packages (pkg_prep_query.execute ()); diff --git a/mod/mod-package-version-details.cxx b/mod/mod-package-version-details.cxx index d1b6dfe..fd8d9a1 100644 --- a/mod/mod-package-version-details.cxx +++ b/mod/mod-package-version-details.cxx @@ -338,8 +338,7 @@ handle (request& rq, response& rs) using query = query; for (auto& b: build_db_->query ( - (query::id.package.name == name && - compare_version_eq (query::id.package.version, ver, true) && + (query::id.package == pkg->id && query::id.configuration.in_range (build_conf_names_->begin (), build_conf_names_->end ())) + diff --git a/mod/options.cli b/mod/options.cli index 051081f..857c2f3 100644 --- a/mod/options.cli +++ b/mod/options.cli @@ -441,7 +441,7 @@ namespace brep class build_task { - // Package repository display name. + // Package repository canonical name. // vector repository | r; }; -- cgit v1.1