From 1f9359a8226c98b14101fd56cfa8a2393fe224f2 Mon Sep 17 00:00:00 2001 From: Timo Pollmeier Date: Tue, 18 Jul 2023 14:38:24 +0200 Subject: [PATCH 1/2] Change: Lock table for each auto-deleted report individually The auto_delete_reports function no longer keeps the access exclusive lock on the reports table for the entire loop over the reports, but instead acquires and releases the lock for each report to delete. This gives other processes, like ones deleting reports manually, a better chance to acquire the lock on the reports table. --- src/manage_sql.c | 75 ++++++++++++++++++++++++++++++++---------------- 1 file changed, 50 insertions(+), 25 deletions(-) diff --git a/src/manage_sql.c b/src/manage_sql.c index 6c580d94d..d8ba2e72d 100644 --- a/src/manage_sql.c +++ b/src/manage_sql.c @@ -18917,15 +18917,7 @@ auto_delete_reports () g_debug ("%s", __func__); - sql_begin_immediate (); - - /* As in delete_report, this prevents other processes from getting the - * report ID. */ - if (sql_int ("SELECT try_exclusive_lock('reports');") == 0) - { - sql_rollback (); - return; - } + GArray *reports_to_delete = g_array_new (TRUE, TRUE, sizeof(report_t)); init_iterator (&tasks, "SELECT id, name," @@ -18970,31 +18962,64 @@ auto_delete_reports () keep); while (next (&reports)) { - int ret; report_t report; report = iterator_int64 (&reports, 0); assert (report); - g_debug ("%s: delete %llu", __func__, report); - ret = delete_report_internal (report); - if (ret == 2) - { - /* Report is in use. */ - g_debug ("%s: %llu is in use", __func__, report); - continue; - } - if (ret) - { - g_warning ("%s: failed to delete %llu (%i)", - __func__, report, ret); - sql_rollback (); - } + g_debug ("%s: %llu to be deleted", __func__, report); + + g_array_append_val (reports_to_delete, report); } cleanup_iterator (&reports); } cleanup_iterator (&tasks); - sql_commit (); + + for (int i = 0; i < reports_to_delete->len; i++) + { + int ret; + report_t report = g_array_index (reports_to_delete, report_t, i); + + sql_begin_immediate (); + + /* As in delete_report, this prevents other processes from getting the + * report ID. */ + if (sql_int ("SELECT try_exclusive_lock('reports');") == 0) + { + g_debug ("%s: could not acquire lock on reports table", __func__); + sql_rollback (); + g_array_free (reports_to_delete, TRUE); + return; + } + + /* Check if report still exists in case another process has deleted it + * in the meantime. */ + if (sql_int ("SELECT count(*) FROM reports WHERE id = %llu", + report) == 0) + { + g_debug ("%s: %llu no longer exists", __func__, report); + sql_rollback (); + continue; + } + + g_debug ("%s: deleting report %llu", __func__, report); + ret = delete_report_internal (report); + if (ret == 2) + { + /* Report is in use. */ + g_debug ("%s: %llu is in use", __func__, report); + continue; + } + if (ret) + { + g_warning ("%s: failed to delete %llu (%i)", + __func__, report, ret); + sql_rollback (); + continue; + } + sql_commit (); + } + g_array_free (reports_to_delete, TRUE); } From bbebfae4ab270c5682b9cc6333a08075217ecd84 Mon Sep 17 00:00:00 2001 From: Timo Pollmeier Date: Fri, 21 Jul 2023 10:08:54 +0200 Subject: [PATCH 2/2] Add missing sql_rollback in auto_delete_reports --- src/manage_sql.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/manage_sql.c b/src/manage_sql.c index d8ba2e72d..5f76b0d90 100644 --- a/src/manage_sql.c +++ b/src/manage_sql.c @@ -19008,6 +19008,7 @@ auto_delete_reports () { /* Report is in use. */ g_debug ("%s: %llu is in use", __func__, report); + sql_rollback (); continue; } if (ret)