Merge pull request #1655

plugins: postgresql fix missing pg_backup_stop() call

CHANGELOG.md

@@ -23,6 +23,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - python-bareos: use socket.create_connection() to allow AF_INET6 [PR #1646]
 - Improve FreeBSD build [PR #1538]
 - core: sql_* add leading space to sql construct [PR #1656]
+- plugins: postgresql fix missing pg_backup_stop() call [PR #1655]
 
 ### Removed
 - plugins: remove old deprecated postgres plugin [PR #1606]
@@ -57,6 +58,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 [PR #1636]: https://github.com/bareos/bareos/pull/1636
 [PR #1637]: https://github.com/bareos/bareos/pull/1637
 [PR #1646]: https://github.com/bareos/bareos/pull/1646
+[PR #1655]: https://github.com/bareos/bareos/pull/1655
 [PR #1656]: https://github.com/bareos/bareos/pull/1656
 [PR #1659]: https://github.com/bareos/bareos/pull/1659
 [unreleased]: https://github.com/bareos/bareos/tree/master

core/src/plugins/filed/python/postgresql/bareos-fd-postgresql.py

@@ -2,7 +2,7 @@
 # -*- coding: utf-8 -*-
 # BAREOS - Backup Archiving REcovery Open Sourced
 #
-# Copyright (C) 2023-2023 Bareos GmbH & Co. KG
+# Copyright (C) 2023-2024 Bareos GmbH & Co. KG
 #
 # This program is Free Software; you can redistribute it and/or
 # modify it under the terms of version three of the GNU Affero General Public
@@ -187,9 +187,11 @@ def __init__(self, plugindef):
         # in end_restore_file(), and this may be mixed up with different files
         self.stat_packets = {}
         self.last_lsn = 0
-        # This will be set to True between select pg_backup_start() and pg_backup_stop().
-        # We backup the cluster files during that time
-        self.full_backup_running = False
+        # True if backup level is Full
+        self.is_full_backup = False
+        # True between select pg_backup_start() and pg_backup_stop(). (while backing up
+        # the cluster)
+        self.is_backup_running = False
         # We will store the `starttime` from `backup_label` here
         self.backup_start_time = None
         # PostgreSQL last backup stop time
@@ -311,7 +313,7 @@ def __build_paths_to_backup(self, start_dir):
         # Now re-add excluded mandatory_subdirs as directory only.
         # But only for Full and `pg_working_dir`
         if (
-            self.full_backup_running
+            self.is_full_backup
             and start_dir == self.cluster_configuration_parameters["data_directory"]
         ):
             for dirname in self.mandatory_subdirs:
@@ -693,7 +695,7 @@ def __complete_backup_job(self):
                 bareosfd.M_ERROR, f"__complete_backup_job unknown failure: {err}\n"
             )
 
-        self.full_backup_running = False
+        self.is_backup_running = False
 
     def __create_db_connection(self):
         """
@@ -876,6 +878,7 @@ def __pg_backup_start(self):
             bareosfd.DebugMessage(150, f"{start_stmt} statement failed: {pg_err}\n")
             raise
 
+        self.is_backup_running = True
         bareosfd.DebugMessage(150, f"Start response LSN: {str(result)}\n")
         bareosfd.DebugMessage(100, "__pg_backup_start() ended\n")
 
@@ -1194,9 +1197,9 @@ def start_backup_job(self):
 
         if chr(self.level) == "F":
             # For Full we backup the PostgreSQL data directory
+            self.is_full_backup = True
             start_dir = self.cluster_configuration_parameters["data_directory"]
             bareosfd.DebugMessage(100, f"data_dir: {start_dir}\n")
-            self.full_backup_running = True
         else:
             # If level is not Full, we only backup WAL files
             # and create a restore object ROP with timestamp information.
@@ -1644,7 +1647,7 @@ def end_backup_file(self):
         if self.paths_to_backup:
             return bareosfd.bRC_More
 
-        if self.full_backup_running:
+        if self.is_full_backup and self.is_backup_running:
             self.__complete_backup_job()
             # Now we can also create the Restore object with the right timestamp
             # We start by ROP so it is the last object in backup and first in restore
@@ -1658,8 +1661,6 @@ def end_backup_file(self):
             self.paths_to_backup += self.virtual_files
             return self.__check_for_wal_files()
 
-        self.__close_db_connection()
-
         return bareosfd.bRC_OK
 
     def end_backup_job(self):
@@ -1668,11 +1669,11 @@ def end_backup_job(self):
         Make sure that db connection was closed in any way,
         especially when job was canceled
         """
-        if self.full_backup_running:
+        bareosfd.DebugMessage(100, "end_backup_job() entry point in Python called\n")
+        # Execute pg_backup_stop() in case of incremental
+        if self.is_backup_running:
             self.__complete_backup_job()
-            self.full_backup_running = False
-        else:
-            self.__close_db_connection()
+        self.__close_db_connection()
         return bareosfd.bRC_OK
 
 

systemtests/tests/py3plug-fd-postgresql/testrunner-default

@@ -210,4 +210,15 @@ else
    estat=1
 fi
 
+if (grep -q "WARN" database/log/postgresql-*.log)
+then
+   echo "Error: Database WARNING found in postgresql log"
+   estat=1
+fi
+if (grep -q "ERR" database/log/postgresql-*.log)
+then
+   echo "Error: Database ERRORS found in postgresql log"
+   estat=1
+fi
+
 end_test