From 3941a9df5c0753ff32f2122423dd5d49676deb61 Mon Sep 17 00:00:00 2001
From: Adam Thorn <alt36@cam.ac.uk>
Date: Fri, 9 Jul 2021 16:07:09 +0100
Subject: [PATCH] Partial fix for behaviour where we see multiple backups for
 one task running at once

In some versions of backup_queue (I think just on splot4 now), we use
backup_log.isrunning as part of the logic to determine if a task should be
enqueued. The problem is that scheduler.pl makes three writes on the table:

1) an insert when the task is queued (a trigger sets isrunning='t' here)
2) an update to set started_processing when the task begins (a trigger
   sets isrunning='f' here!!!!)
3) an update to set ended_processing when the task finishes (a trigger
   again sets isrunning='f' here)

Thus, being careful to only set isrunning='f' when a backup task is finished
(i.e. when we set ended_processing=now() in scheduler.pl) seems sensible, and
empirically does seem to lead to the right backup_queue without duplicates.

This commit will only affect new setups of backup servers; the change has been
deployed to live servers with an ad hoc script I've run.

I think we only see this on splot4 because it has a very different definition of
the backup_queue view to a) the one defined in this file, b) the one that's on
all the other backup servers. If I just try to replace the view on splot4, though,
any attempt to select from it just times out so there may be other relations on
splot4 that need updating too.

NB the obvious thing missing on splot4 is

WHERE ((backup_log.backup_task_id = a.backup_task_id) AND (backup_log.ended_processing IS NULL))) < 1))

which feels like a hack but nonetheless ensures in practice that we don't get
duplicate queued tasks.
---
 ROOT/usr/lib/chem-zfs-backup-server/DB/database.sql | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/ROOT/usr/lib/chem-zfs-backup-server/DB/database.sql b/ROOT/usr/lib/chem-zfs-backup-server/DB/database.sql
index 1100e1a..4eed570 100644
--- a/ROOT/usr/lib/chem-zfs-backup-server/DB/database.sql
+++ b/ROOT/usr/lib/chem-zfs-backup-server/DB/database.sql
@@ -376,9 +376,12 @@ CREATE FUNCTION update_ended_processing() RETURNS trigger
 declare
  
 begin
- update backup_task set last_ended=new.ended_processing where backup_task_id=new.backup_task_id and last_ended<new.ended_processing;
- update backup_task set isrunning='f' where backup_task_id=new.backup_task_id and isrunning;
-  return new;
+ -- NB we also update the backup_log table to set started_processing; we only want to do the following updates if we have ended_processing
+ if new.ended_processing is not null then
+   update backup_task set last_ended=new.ended_processing where backup_task_id=new.backup_task_id;
+   update backup_task set isrunning='f' where backup_task_id=new.backup_task_id and isrunning;
+ end if;
+ return new;
 end
 $$;
 
-- 
GitLab