From 9b25cfb730aa3fe969bc3509e8069480bc2414d6 Mon Sep 17 00:00:00 2001 From: Jairo Llopis Date: Thu, 21 Feb 2019 09:49:29 +0000 Subject: [PATCH] update: Parallel DB locks watcher With this patch, if you update a database while another Odoo instance is running (such as i.e. a production instance), the production instance will not be locked just because there's a DB lock. DB locks can happen i.e. when 2 or more transactions are happening in parallel and one of them wants to modify data in a field while another one is modifying the field itself. For example, imagine that a user is modifying a `res.partner`'s name, while another update process is adding a `UNIQUE` constraint in the `name` field of the `res_partner` table. This would produce a deadlock where each transaction is waiting for the other one to finish, and thus both the production instance and the update instance would be locked infinitely until one of them is aborted. You cannot detect such problem with common tools such as timeouts, because there still exists the possibility of a query being slow without actually being locked, like when you update an addon that has a pre-update migration script that performs lots of work, or when your queries or server are not optimized and perform slowly. So, the only way to detect deadlocks is by issuing a separate DB cursor that is not protected by a transaction and that watches other cursors' transactions and their locks. With this change, this is what happens now behind the scenes: - The DB lock watcher process is spawned in background using a separate watcher cursor and watches for locks. - The foreground process starts updating Odoo. - If a lock is detected, the update process is aborted, giving priority to the other cursors. This is by design because your production users have priority always, and all that would happen is that the update transaction would be rolled back, so you can just try updating again later. - A couple of CLI parameters allow you to modify the watcher behavior, or completely disable it. Keep in mind that an update in Odoo issues several commits, so before starting the parallel update you must make sure the production server is running in a mode that won't reload workers, and if using Odoo < 10, that won't launch cron jobs. --- .pre-commit-config.yaml | 10 +-- README.rst | 31 +++++++-- click_odoo_contrib/update.py | 128 +++++++++++++++++++++++++++++++++-- 3 files changed, 151 insertions(+), 18 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 36dfad0..c7d6eb8 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -4,7 +4,7 @@ repos: rev: 18.9b0 hooks: - id: black - language_version: python3.6 + language_version: python3 - repo: https://github.com/pre-commit/pre-commit-hooks rev: v1.4.0 hooks: @@ -14,19 +14,19 @@ repos: - id: debug-statements - id: flake8 additional_dependencies: ["flake8-bugbear == 18.8.0"] - language_version: python3.6 + language_version: python3 - repo: https://github.com/asottile/pyupgrade rev: v1.6.1 hooks: - id: pyupgrade - language_version: python3.6 + language_version: python3 - repo: https://github.com/asottile/seed-isort-config rev: v1.3.0 hooks: - id: seed-isort-config - language_version: python3.6 + language_version: python3 - repo: https://github.com/pre-commit/mirrors-isort rev: v4.3.4 hooks: - id: isort - language_version: python3.6 + language_version: python3 diff --git a/README.rst b/README.rst index 1f95dab..668e8c6 100644 --- a/README.rst +++ b/README.rst @@ -183,14 +183,31 @@ click-odoo-update (beta) update based on a hash of their file content, compared to the hashes stored in the database. + It allows updating in parallel while another Odoo instance is still + running against the same database, by using a watcher that aborts the + update in case a DB lock happens. + Options: - -c, --config FILE ... - -d, --database TEXT ... - ... - --i18n-overwrite Overwrite existing translations - --update-all Force a complete upgrade (-u base) - --if-exists Don't report error if database doesn't exist - --help Show this message and exit. + -c, --config FILE Specify the Odoo configuration file. Other ways + to provide it are with the ODOO_RC or + OPENERP_SERVER environment variables, or + ~/.odoorc (Odoo >= 10) or ~/.openerp_serverrc. + --addons-path TEXT Specify the addons path. If present, this + parameter takes precedence over the addons path + provided in the Odoo configuration file. + -d, --database TEXT Specify the database name. If present, this + parameter takes precedence over the database + provided in the Odoo configuration file. + --log-level TEXT Specify the logging level. Accepted values + depend on the Odoo version, and include debug, + info, warn, error. [default: info] + --logfile FILE Specify the log file. + --i18n-overwrite Overwrite existing translations + --update-all Force a complete upgrade (-u base) + --if-exists Don't report error if database doesn't exist + --watcher-max-seconds FLOAT Max DB lock seconds allowed before aborting the + update process. Default: 0 (disabled). + --help Show this message and exit. click-odoo-upgrade (deprecated, see click-odoo-update) ------------------------------------------------------ diff --git a/click_odoo_contrib/update.py b/click_odoo_contrib/update.py index bdf551a..13bf274 100644 --- a/click_odoo_contrib/update.py +++ b/click_odoo_contrib/update.py @@ -5,10 +5,14 @@ import json import logging import os -from contextlib import contextmanager +import threading +from contextlib import closing, contextmanager +from datetime import timedelta +from time import sleep import click import click_odoo +import psycopg2 from click_odoo import OdooEnvironment, odoo from ._addon_hash import addon_hash @@ -19,6 +23,89 @@ PARAM_INSTALLED_CHECKSUMS = "module_auto_update.installed_checksums" PARAM_EXCLUDE_PATTERNS = "module_auto_update.exclude_patterns" DEFAULT_EXCLUDE_PATTERNS = "*.pyc,*.pyo,i18n/*.pot,i18n_extra/*.pot,static/*" +watching = True + + +def _db_lock_watcher(database, max_seconds): + """Watch DB while another process is updating Odoo. + + This method will query :param:`database` to see if there are DB locks. + If a lock longer than :param:`max_seconds` is detected, it will be + terminated and an exception will be raised. + + :param str database: + Name of the database that is being updated in parallel. + + :param float max_seconds: + Max length of DB lock allowed. + """ + _logger.info("Starting DB lock watcher") + global watching + beat = max_seconds / 3 + max_td = timedelta(seconds=max_seconds) + own_pid_query = "SELECT pg_backend_pid()" + # SQL explained in https://wiki.postgresql.org/wiki/Lock_Monitoring + locks_query = """ + SELECT + pg_stat_activity.datname, + pg_class.relname, + pg_locks.transactionid, + pg_locks.mode, + pg_locks.granted, + pg_stat_activity.usename, + pg_stat_activity.query, + pg_stat_activity.query_start, + AGE(NOW(), pg_stat_activity.query_start) AS age, + pg_stat_activity.pid + FROM + pg_stat_activity + JOIN pg_locks ON pg_locks.pid = pg_stat_activity.pid + JOIN pg_class ON pg_class.oid = pg_locks.relation + WHERE + NOT pg_locks.granted + AND pg_stat_activity.datname = %s + ORDER BY pg_stat_activity.query_start + """ + # See https://stackoverflow.com/a/35319598/1468388 + stop_query = "SELECT pg_cancel_backend(%s)" + if odoo.release.version_info < (9, 0): + params = {"dsn": odoo.sql_db.dsn(database)[1]} + else: + params = odoo.sql_db.connection_info_for(database)[1] + # Need a separate raw psycopg2 cursor without transactioning to avoid + # weird concurrency errors; this cursor will only trigger SELECTs, and + # it needs to access current Postgres server status, monitoring other + # transactions' status, so running inside a normal, transactioned, + # Odoo cursor would block such monitoring and, after all, offer no + # additional protection + with closing(psycopg2.connect(**params)) as watcher_conn: + watcher_conn.set_isolation_level(psycopg2.extensions.ISOLATION_LEVEL_AUTOCOMMIT) + while watching: + # Wait some time before checking locks + sleep(beat) + # Ensure no long blocking queries happen + with closing( + watcher_conn.cursor(cursor_factory=psycopg2.extras.DictCursor) + ) as watcher_cr: + if _logger.level <= logging.DEBUG: + watcher_cr.execute(own_pid_query) + watcher_pid = watcher_cr.fetchone()[0] + _logger.debug( + "DB lock watcher running with postgres PID %d", watcher_pid + ) + watcher_cr.execute(locks_query, (database,)) + locks = watcher_cr.fetchall() + if locks: + _logger.warning("%d locked queries found", len(locks)) + _logger.info("Query details: %r", locks) + for row in locks: + if row["age"] > max_td: + _logger.warning( + "Long locked query detected; aborting update cursor..." + ) + # Terminate the query to abort the parallel update + watcher_cr.execute(stop_query, (row["pid"],)) + return def _get_param(cr, key, default=None): @@ -53,6 +140,7 @@ def _save_installed_checksums(cr): for (module_name,) in cr.fetchall(): checksums[module_name] = _get_checksum_dir(cr, module_name) _set_param(cr, PARAM_INSTALLED_CHECKSUMS, json.dumps(checksums)) + _logger.info("Database updated, new checksums stored") def _get_checksum_dir(cr, module_name): @@ -70,11 +158,10 @@ def _get_checksum_dir(cr, module_name): return checksum_dir -@contextmanager -def OdooEnvironmentWithUpdate(database, ctx, **kwargs): +def _update_db(database, update_all, i18n_overwrite): conn = odoo.sql_db.db_connect(database) to_update = odoo.tools.config["update"] - if ctx.params["update_all"]: + if update_all: to_update["base"] = 1 else: with conn.cursor() as cr: @@ -88,7 +175,7 @@ def OdooEnvironmentWithUpdate(database, ctx, **kwargs): "Updating addons for their hash changed: %s.", ",".join(to_update.keys()), ) - if ctx.params["i18n_overwrite"]: + if i18n_overwrite: odoo.tools.config["overwrite_existing_translations"] = True if odoo.release.version_info[0] < 10: Registry = odoo.modules.registry.RegistryManager @@ -97,6 +184,24 @@ def OdooEnvironmentWithUpdate(database, ctx, **kwargs): Registry.new(database, update_module=True) with conn.cursor() as cr: _save_installed_checksums(cr) + + +@contextmanager +def OdooEnvironmentWithUpdate(database, ctx, **kwargs): + global watching + # Watch for database locks while Odoo updates + if ctx.params["watcher_max_seconds"] > 0: + watcher = threading.Thread( + target=_db_lock_watcher, args=(database, ctx.params["watcher_max_seconds"]) + ) + watcher.daemon = True + watcher.start() + # Update Odoo datatabase + try: + _update_db(database, ctx.params["update_all"], ctx.params["i18n_overwrite"]) + finally: + watching = False + # If we get here, the database has been updated with OdooEnvironment(database) as env: yield env @@ -113,10 +218,21 @@ def OdooEnvironmentWithUpdate(database, ctx, **kwargs): @click.option( "--if-exists", is_flag=True, help="Don't report error if database doesn't exist" ) -def main(env, i18n_overwrite, update_all, if_exists): +@click.option( + "--watcher-max-seconds", + default=0, + type=float, + help="Max DB lock seconds allowed before aborting the update process. " + "Default: 0 (disabled).", +) +def main(env, i18n_overwrite, update_all, if_exists, watcher_max_seconds): """ Update an Odoo database (odoo -u), automatically detecting addons to update based on a hash of their file content, compared to the hashes stored in the database. + + It allows updating in parallel while another Odoo instance is still + running against the same database, by using a watcher that aborts the + update in case a DB lock happens. """ if not env: msg = "Database does not exist"