Change: Improve table locking with timeout + retry

When trying to acquire the report table lock with retries, do not use
NOWAIT and sleep (), but instead use the lock_timeout setting in the
current transaction.
The timeout is adjusted to be below the deadlock_timeout and the number
of retries is raised accordingly.

This allows waiting processes to acquire the lock as soon as it becomes
available, reducing wait times for transactions blocked by fast other
transactions.

src/manage_pg.c

@@ -342,6 +342,18 @@ manage_create_sql_functions ()
        " END;"
        "$$ language 'plpgsql';");
 
+  sql ("CREATE OR REPLACE FUNCTION try_exclusive_lock_wait (regclass)"
+       " RETURNS integer AS $$"
+       " BEGIN"
+       "   EXECUTE 'LOCK TABLE '"
+       "           || quote_ident_split($1::text)"
+       "           || ' IN ACCESS EXCLUSIVE MODE;';"
+       "   RETURN 1;"
+       " EXCEPTION WHEN lock_not_available THEN"
+       "   RETURN 0;"
+       " END;"
+       "$$ language 'plpgsql';");
+
   if (sql_int ("SELECT EXISTS (SELECT * FROM information_schema.tables"
                "               WHERE table_catalog = '%s'"
                "               AND table_schema = 'public'"

src/manage_sql.c

@@ -93,12 +93,12 @@
  *        LOCK TABLE .. IN ACCESS EXLUSIVE MODE NOWAIT
  *        statements.
  */
-#define LOCK_RETRIES 16
+#define LOCK_RETRIES 64
 
 /**
- * @brief Time of delay between two lock retries.
+ * @brief Timeout for trying to acquire a lock in milliseconds.
  */
-#define LOCK_RETRY_DELAY 2
+#define LOCK_TIMEOUT 500
 
 #ifdef DEBUG_FUNCTION_NAMES
 #include <dlfcn.h>
@@ -25497,11 +25497,10 @@ delete_report (const char *report_id, int dummy)
    * If the report is running already then delete_report_internal will
    * ROLLBACK. */
   lock_retries = LOCK_RETRIES;
-  lock_ret = sql_int ("SELECT try_exclusive_lock('reports');");
+  lock_ret = sql_table_lock_wait ("reports", LOCK_TIMEOUT);
   while ((lock_ret == 0) && (lock_retries > 0))
     {
-      sleep(LOCK_RETRY_DELAY);
-      lock_ret = sql_int ("SELECT try_exclusive_lock('reports');");
+      lock_ret = sql_table_lock_wait ("reports", LOCK_TIMEOUT);
       lock_retries--;
     }
   if (lock_ret == 0)
@@ -31233,11 +31232,10 @@ delete_task_lock (task_t task, int ultimate)
    * If the task is already active then delete_report (via delete_task)
    * will fail and rollback. */
   lock_retries = LOCK_RETRIES;
-  lock_ret = sql_int ("SELECT try_exclusive_lock('reports');");
+  lock_ret = sql_table_lock_wait ("reports", LOCK_TIMEOUT);
   while ((lock_ret == 0) && (lock_retries > 0))
     {
-      sleep(LOCK_RETRY_DELAY);
-      lock_ret = sql_int ("SELECT try_exclusive_lock('reports');");
+      lock_ret = sql_table_lock_wait ("reports", LOCK_TIMEOUT);
       lock_retries--;
     }
   if (lock_ret == 0)
@@ -31413,11 +31411,10 @@ request_delete_task_uuid (const char *task_id, int ultimate)
                * If the task is running already then delete_task will lead to
                * ROLLBACK. */
               lock_retries = LOCK_RETRIES;
-              lock_ret = sql_int ("SELECT try_exclusive_lock('reports');");
+              lock_ret = sql_table_lock_wait ("reports", LOCK_TIMEOUT);
               while ((lock_ret == 0) && (lock_retries > 0))
                 {
-                  sleep(LOCK_RETRY_DELAY);
-                  lock_ret = sql_int ("SELECT try_exclusive_lock('reports');");
+                  lock_ret = sql_table_lock_wait ("reports", LOCK_TIMEOUT);
                   lock_retries--;
                 }
               if (lock_ret == 0)

src/sql.h

@@ -129,6 +129,9 @@ sql_commit ();
 void
 sql_rollback ();
 
+int
+sql_table_lock_wait (const char *, int);
+
 /* Iterators. */
 
 /* These functions are for "internal" use.  They may only be accessed by code

src/sql_pg.c

@@ -602,6 +602,27 @@ sql_rollback ()
   sql ("ROLLBACK;");
 }
 
+/**
+ * Try to lock a table, timing out after a given time.
+ *
+ * @param[in]  table         The table to lock.
+ * @param[in]  lock_timeout  The lock timeout in milliseconds, 0 for unlimited.
+ * 
+ * @return 1 if locked, 0 if failed / timed out.
+ */
+int
+sql_table_lock_wait (const char *table, int lock_timeout)
+{
+  int old_lock_timeout = sql_int ("SHOW lock_timeout;");
+  sql ("SET LOCAL lock_timeout = %d;", lock_timeout);
+
+  // This requires the gvmd functions to be defined first.
+  int ret = sql_int ("SELECT try_exclusive_lock_wait (%s);", table);
+
+  sql ("SET LOCAL lock_timeout = %d;", old_lock_timeout);
+  return ret;
+}
+
 
 /* Iterators. */