From 0b6b57f9acaa2ec648bf582ff67851331f8e6eef Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Mon, 14 Mar 2016 14:38:45 +0300 Subject: Use serializable transaction isolation level --- brep/buildfile | 9 +++--- brep/database-module | 55 ++++++++++++++++++++++++++++++++++++ brep/database-module.cxx | 50 ++++++++++++++++++++++++++++++++ brep/database.cxx | 12 ++++---- brep/mod-package-details | 7 ++--- brep/mod-package-details.cxx | 10 +++---- brep/mod-package-search | 7 ++--- brep/mod-package-search.cxx | 14 ++++----- brep/mod-package-version-details | 7 ++--- brep/mod-package-version-details.cxx | 10 +++---- brep/mod-repository-details | 7 ++--- brep/mod-repository-details.cxx | 10 +++---- brep/mod-repository-root.cxx | 2 +- brep/module | 5 ++-- brep/module.cxx | 11 ++++++-- brep/options.cli | 25 ++++++++++------ 16 files changed, 174 insertions(+), 67 deletions(-) create mode 100644 brep/database-module create mode 100644 brep/database-module.cxx (limited to 'brep') diff --git a/brep/buildfile b/brep/buildfile index 360f82d..4caf93a 100644 --- a/brep/buildfile +++ b/brep/buildfile @@ -42,17 +42,18 @@ import libs += libstudxml%lib{studxml} gen = {hxx ixx cxx}{ options } src = \ + {hxx cxx}{ database } \ + {hxx cxx}{ database-module } \ {hxx cxx}{ diagnostics } \ - {hxx cxx}{ module } \ - {hxx }{ options-types } \ {hxx cxx}{ mod-package-details } \ {hxx cxx}{ mod-package-search } \ {hxx cxx}{ mod-package-version-details } \ - {hxx cxx}{ page } \ {hxx cxx}{ mod-repository-details } \ {hxx cxx}{ mod-repository-root } \ + {hxx cxx}{ module } \ + {hxx }{ options-types } \ + {hxx cxx}{ page } \ { cxx}{ services } \ - {hxx cxx}{ database } \ {hxx cxx}{ types-parsers } \ {hxx }{ wrapper-traits } \ ../web/{hxx cxx}{ mime-url-encoding } \ diff --git a/brep/database-module b/brep/database-module new file mode 100644 index 0000000..64d5aaf --- /dev/null +++ b/brep/database-module @@ -0,0 +1,55 @@ +// file : brep/database-module -*- C++ -*- +// copyright : Copyright (c) 2014-2016 Code Synthesis Ltd +// license : MIT; see accompanying LICENSE file + +#ifndef BREP_DATABASE_MODULE +#define BREP_DATABASE_MODULE + +#include // database + +#include +#include + +#include +#include + +namespace brep +{ + // A module that utilises the database. Specifically, it will retry the + // request in the face of recoverable database failures (deadlock, loss of + // connection, etc) up to a certain number of times. + // + class database_module: public module + { + protected: + database_module () = default; + + // Create a shallow copy (handling instance) if initialized and a deep + // copy (context exemplar) otherwise. + // + explicit + database_module (const database_module&); + + // Required to avoid getting warning from clang that + // database_module::init() hides module::init() virtual functions. This + // way all functions get to the same scope and become overloaded set. + // + using module::init; + + void + init (const options::db&); + + virtual bool + handle (request&, response&) = 0; + + protected: + size_t retry_; + shared_ptr db_; + + private: + virtual bool + handle (request&, response&, log&); + }; +} + +#endif // BREP_DATABASE_MODULE diff --git a/brep/database-module.cxx b/brep/database-module.cxx new file mode 100644 index 0000000..630fd89 --- /dev/null +++ b/brep/database-module.cxx @@ -0,0 +1,50 @@ +// file : brep/database-module.cxx -*- C++ -*- +// copyright : Copyright (c) 2014-2016 Code Synthesis Ltd +// license : MIT; see accompanying LICENSE file + +#include + +#include + +#include +#include + +namespace brep +{ + // 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. + // + database_module:: + database_module (const database_module& r) + : module (r), + retry_ (r.retry_), + db_ (r.initialized_ ? r.db_ : nullptr) + { + } + + void database_module:: + init (const options::db& o) + { + retry_ = o.db_retry (); + db_ = shared_database (o); + } + + bool database_module:: + handle (request& rq, response& rs, log& l) + try + { + return module::handle (rq, rs, l); + } + catch (const odb::recoverable& e) + { + if (retry_-- > 0) + { + MODULE_DIAG; + l1 ([&]{trace << e.what () << "; " << retry_ + 1 << " retries left";}); + throw retry (); + } + + throw; + } +} diff --git a/brep/database.cxx b/brep/database.cxx index 17605c5..1f70881 100644 --- a/brep/database.cxx +++ b/brep/database.cxx @@ -41,11 +41,13 @@ namespace brep } shared_ptr d ( - make_shared (o.db_user (), - o.db_password (), - o.db_name (), - o.db_host (), - o.db_port ())); + make_shared ( + o.db_user (), + o.db_password (), + o.db_name (), + o.db_host (), + o.db_port (), + "options='-c default_transaction_isolation=serializable'")); databases[o] = d; return d; diff --git a/brep/mod-package-details b/brep/mod-package-details index 0a86fc2..4dc6b18 100644 --- a/brep/mod-package-details +++ b/brep/mod-package-details @@ -5,17 +5,15 @@ #ifndef BREP_MOD_PACKAGE_DETAILS #define BREP_MOD_PACKAGE_DETAILS -#include // database - #include #include -#include #include +#include namespace brep { - class package_details: public module + class package_details: public database_module { public: package_details () = default; @@ -38,7 +36,6 @@ namespace brep private: shared_ptr options_; - shared_ptr db_; }; } diff --git a/brep/mod-package-details.cxx b/brep/mod-package-details.cxx index a5c2e3e..10bd72c 100644 --- a/brep/mod-package-details.cxx +++ b/brep/mod-package-details.cxx @@ -18,7 +18,6 @@ #include #include #include -#include #include using namespace odb::core; @@ -30,9 +29,8 @@ using namespace brep::cli; // brep::package_details:: package_details (const package_details& r) - : module (r), - options_ (r.initialized_ ? r.options_ : nullptr), - db_ (r.initialized_ ? r.db_ : nullptr) + : database_module (r), + options_ (r.initialized_ ? r.options_ : nullptr) { } @@ -44,10 +42,10 @@ init (scanner& s) options_ = make_shared ( s, unknown_mode::fail, unknown_mode::fail); + database_module::init (*options_); + if (options_->root ().empty ()) options_->root (dir_path ("/")); - - db_ = shared_database (*options_); } template diff --git a/brep/mod-package-search b/brep/mod-package-search index 1730be4..bb762cd 100644 --- a/brep/mod-package-search +++ b/brep/mod-package-search @@ -5,17 +5,15 @@ #ifndef BREP_MOD_PACKAGE_SEARCH #define BREP_MOD_PACKAGE_SEARCH -#include // database - #include #include -#include #include +#include namespace brep { - class package_search: public module + class package_search: public database_module { public: package_search () = default; @@ -38,7 +36,6 @@ namespace brep private: shared_ptr options_; - shared_ptr db_; }; } diff --git a/brep/mod-package-search.cxx b/brep/mod-package-search.cxx index 77a06b6..9769783 100644 --- a/brep/mod-package-search.cxx +++ b/brep/mod-package-search.cxx @@ -21,7 +21,6 @@ #include #include #include -#include #include using namespace odb::core; @@ -33,9 +32,8 @@ using namespace brep::cli; // brep::package_search:: package_search (const package_search& r) - : module (r), - options_ (r.initialized_ ? r.options_ : nullptr), - db_ (r.initialized_ ? r.db_ : nullptr) + : database_module (r), + options_ (r.initialized_ ? r.options_ : nullptr) { } @@ -47,11 +45,11 @@ init (scanner& s) options_ = make_shared ( s, unknown_mode::fail, unknown_mode::fail); + database_module::init (*options_); + if (options_->root ().empty ()) options_->root (dir_path ("/")); - db_ = shared_database (*options_); - // Check that the database schema matches the current one. It's enough to // perform the check in just a single module implementation (and we don't // do in the dispatcher because it doesn't use the database). @@ -94,7 +92,8 @@ handle (request& rq, response& rs) try { name_value_scanner s (rq.parameters ()); - params = params::package_search (s, unknown_mode::fail, unknown_mode::fail); + params = params::package_search ( + s, unknown_mode::fail, unknown_mode::fail); } catch (const unknown_argument& e) { @@ -107,6 +106,7 @@ handle (request& rq, response& rs) ? "" : "?q=" + web::mime_url_encode (squery)); + static const string title ("Packages"); xml::serializer s (rs.content (), title); diff --git a/brep/mod-package-version-details b/brep/mod-package-version-details index cfbcf94..a463511 100644 --- a/brep/mod-package-version-details +++ b/brep/mod-package-version-details @@ -5,17 +5,15 @@ #ifndef BREP_MOD_PACKAGE_VERSION_DETAILS #define BREP_MOD_PACKAGE_VERSION_DETAILS -#include // database - #include #include -#include #include +#include namespace brep { - class package_version_details: public module + class package_version_details: public database_module { public: package_version_details () = default; @@ -41,7 +39,6 @@ namespace brep private: shared_ptr options_; - shared_ptr db_; }; } diff --git a/brep/mod-package-version-details.cxx b/brep/mod-package-version-details.cxx index 992b829..21da41f 100644 --- a/brep/mod-package-version-details.cxx +++ b/brep/mod-package-version-details.cxx @@ -18,7 +18,6 @@ #include #include #include -#include #include using namespace std; @@ -31,9 +30,8 @@ using namespace brep::cli; // brep::package_version_details:: package_version_details (const package_version_details& r) - : module (r), - options_ (r.initialized_ ? r.options_ : nullptr), - db_ (r.initialized_ ? r.db_ : nullptr) + : database_module (r), + options_ (r.initialized_ ? r.options_ : nullptr) { } @@ -45,10 +43,10 @@ init (scanner& s) options_ = make_shared ( s, unknown_mode::fail, unknown_mode::fail); + database_module::init (*options_); + if (options_->root ().empty ()) options_->root (dir_path ("/")); - - db_ = shared_database (*options_); } bool brep::package_version_details:: diff --git a/brep/mod-repository-details b/brep/mod-repository-details index 411d9e6..49ad629 100644 --- a/brep/mod-repository-details +++ b/brep/mod-repository-details @@ -5,17 +5,15 @@ #ifndef BREP_MOD_REPOSITORY_DETAILS #define BREP_MOD_REPOSITORY_DETAILS -#include // database - #include #include -#include #include +#include namespace brep { - class repository_details: public module + class repository_details: public database_module { public: repository_details () = default; @@ -38,7 +36,6 @@ namespace brep private: shared_ptr options_; - shared_ptr db_; }; } diff --git a/brep/mod-repository-details.cxx b/brep/mod-repository-details.cxx index f040be6..b0e2b95 100644 --- a/brep/mod-repository-details.cxx +++ b/brep/mod-repository-details.cxx @@ -24,7 +24,6 @@ #include #include #include -#include #include using namespace std; @@ -37,9 +36,8 @@ using namespace brep::cli; // brep::repository_details:: repository_details (const repository_details& r) - : module (r), - options_ (r.initialized_ ? r.options_ : nullptr), - db_ (r.initialized_ ? r.db_ : nullptr) + : database_module (r), + options_ (r.initialized_ ? r.options_ : nullptr) { } @@ -51,11 +49,11 @@ init (scanner& s) options_ = make_shared ( s, unknown_mode::fail, unknown_mode::fail); + database_module::init (*options_); + if (options_->root ().empty ()) options_->root (dir_path ("/")); - db_ = shared_database (*options_); - tzset (); // To use butl::to_stream() later on. } diff --git a/brep/mod-repository-root.cxx b/brep/mod-repository-root.cxx index 7c27a60..7d0adc9 100644 --- a/brep/mod-repository-root.cxx +++ b/brep/mod-repository-root.cxx @@ -40,7 +40,7 @@ namespace brep cookies () {return request_.cookies ();} virtual istream& - content () {return request_.content ();} + content (bool buffer) {return request_.content (buffer);} private: request& request_; diff --git a/brep/module b/brep/module index 52106cd..3796399 100644 --- a/brep/module +++ b/brep/module @@ -62,7 +62,7 @@ namespace brep const basic_mark info (severity::info, \ this->log_writer_, \ __PRETTY_FUNCTION__); \ - const basic_mark trace (severity::info, \ + const basic_mark trace (severity::trace, \ this->log_writer_, \ __PRETTY_FUNCTION__) @@ -76,7 +76,8 @@ namespace brep // Trace verbosity level. // // 0 - tracing disabled. - // 1 - @@ TODO: document + // 1 - brief information regarding irregular situations, which not being + // an error can be of some interest. // 2 - @@ TODO: document // // While uint8 is more than enough, use uint16 for the ease of printing. diff --git a/brep/module.cxx b/brep/module.cxx index 1257d82..40acad9 100644 --- a/brep/module.cxx +++ b/brep/module.cxx @@ -201,7 +201,9 @@ namespace brep // Read brep::module configuration. // - static option_descriptions od (convert (options::module::description ())); + static option_descriptions od ( + convert (options::module::description ())); + name_values mo (filter (opts, od)); name_value_scanner s (mo); options::module o (s, cli::unknown_mode::fail, cli::unknown_mode::fail); @@ -311,7 +313,12 @@ namespace brep // Considered using lambda for mapping but looks too verbose while can // be a bit safer in runtime. // - static int s[] = {APLOG_ERR, APLOG_WARNING, APLOG_INFO, APLOG_TRACE1}; + // Use APLOG_INFO (as opposed to APLOG_TRACE1) as a mapping for + // severity::trace. "LogLevel trace1" configuration directive switches + // on the avalanche of log messages from various modules. Would be good + // to avoid wading through them. + // + static int s[] = {APLOG_ERR, APLOG_WARNING, APLOG_INFO, APLOG_INFO}; for (const auto& e: d) { diff --git a/brep/options.cli b/brep/options.cli index f3d26d0..bcb335e 100644 --- a/brep/options.cli +++ b/brep/options.cli @@ -54,15 +54,16 @@ namespace brep string db-name = "brep" { "", - "Database name. If not specified, then '\cb{brep}' is used by default." + "Database name. If not specified, then '\cb{brep}' is used by + default." } string db-host { "", - "Database host name, address, or socket. If not specified, then connect - to \cb{localhost} using the operating system-default mechanism - (Unix-domain socket, etc)." + "Database host name, address, or socket. If not specified, then + connect to \cb{localhost} using the operating system-default + mechanism (Unix-domain socket, etc)." } uint16_t db-port = 0 @@ -70,6 +71,14 @@ namespace brep "", "Database port number. If not specified, the default port is used." } + + size_t db-retry = 10 + { + "", + "The maximum number of times to retry database transactions in the + face of recoverable failures (deadlock, loss of connection, etc). The + default is 10." + } }; class page @@ -84,10 +93,10 @@ namespace brep vector menu; { "", - "Web page menu. Each entry is displayed in the page header in the order - specified and aligned to the right edge. A link target that starts - with '\cb{/}' or contains '\cb{:}' is used as is. Otherwise, it is - prefixed with the repository web interface root." + "Web page menu. Each entry is displayed in the page header in the + order specified and aligned to the right edge. A link target that + starts with '\cb{/}' or contains '\cb{:}' is used as is. Otherwise, + it is prefixed with the repository web interface root." } }; -- cgit v1.1