From 3309a1fbb9ef2ec510575292f162a5390787b234 Mon Sep 17 00:00:00 2001
From: Timo Pollmeier <timo.pollmeier@greenbone.net>
Date: Thu, 2 Nov 2023 14:22:08 +0100
Subject: [PATCH] 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  | 12 ++++++++++++
 src/manage_sql.c | 21 +++++++++------------
 src/sql.h        |  3 +++
 src/sql_pg.c     | 21 +++++++++++++++++++++
 4 files changed, 45 insertions(+), 12 deletions(-)

diff --git a/src/manage_pg.c b/src/manage_pg.c
index df9741fedb..928e271427 100644
--- a/src/manage_pg.c
+++ b/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'"
diff --git a/src/manage_sql.c b/src/manage_sql.c
index c57d597b6b..2c9a2056ac 100644
--- a/src/manage_sql.c
+++ b/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)
diff --git a/src/sql.h b/src/sql.h
index 65600bf485..b0a225eff6 100644
--- a/src/sql.h
+++ b/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
diff --git a/src/sql_pg.c b/src/sql_pg.c
index 0aec109972..cf8ed82c87 100644
--- a/src/sql_pg.c
+++ b/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. */