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

@@ -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

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)
 ------------------------------------------------------

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"