Skip to content

Commit

Permalink
Avoid connecting to PostgreSQL while reading a backup.info file
Browse files Browse the repository at this point in the history
Prevent Barman from opening a PostgreSQL transaction that lasts until
termination, even in operations that should not require to speak with
Postgres (i.e. list-backup or recover).

This fixes a regression introduced with systemid check code.

Signed-off-by: Marco Nenciarini <marco.nenciarini@2ndquadrant.it>
Signed-off-by: Leonardo Cecchi <leonardo.cecchi@2ndquadrant.it>
  • Loading branch information
mnencia committed Dec 4, 2019
1 parent 616c776 commit dde2d12
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 2 deletions.
1 change: 1 addition & 0 deletions barman/backup.py
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,7 @@ def backup(self, wait=False, wait_timeout=None):
backup_info = LocalBackupInfo(
self.server,
backup_id=datetime.datetime.now().strftime('%Y%m%dT%H%M%S'))
backup_info.set_attribute('systemid', self.server.systemid)
backup_info.save()
self.backup_cache_add(backup_info)
output.info(
Expand Down
1 change: 0 additions & 1 deletion barman/infofile.py
Original file line number Diff line number Diff line change
Expand Up @@ -596,7 +596,6 @@ def __init__(self, server, info_file=None, backup_id=None, **kwargs):
'both info_file and backup_id parameters are set')
self.backup_id = backup_id
self.filename = self.get_filename()
self.systemid = server.systemid
# Check if a backup info file for a given server and a given ID
# already exists. If so load the values from the file.
if os.path.exists(self.filename):
Expand Down
26 changes: 25 additions & 1 deletion tests/test_infofile.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@

from barman.infofile import (BackupInfo, Field, FieldListFile, LocalBackupInfo,
WalFileInfo, load_datetime_tz)
from testing_helpers import build_backup_manager, build_mocked_server
from testing_helpers import (build_backup_manager, build_mocked_server,
build_real_server)


BASE_BACKUP_INFO = """backup_label=None
begin_offset=40
Expand Down Expand Up @@ -579,3 +581,25 @@ def test_xlog_segment_size(self, tmpdir):
# load the data from the backup.info file
b_info = LocalBackupInfo(server, info_file=infofile.strpath)
assert b_info.xlog_segment_size == 1 << 24

@mock.patch('barman.postgres.PostgreSQLConnection.connect')
def test_backupinfo_load(self, connect_mock, tmpdir):
server = build_real_server(
main_conf={
'basebackups_directory': tmpdir.strpath
},
)

# Build a fake backup info and try to load id, to ensure that we won't
# need a PostgreSQL connection to do that
backup_dir = tmpdir.mkdir('fake_backup_id')
info_file = backup_dir.join('backup.info')
info_file.write(BASE_BACKUP_INFO)

# Monkey patch the PostgreSQL connection function to raise a
# RuntimeError
connect_mock.side_effect = RuntimeError

# The following constructor will raise a RuntimeError if we are
# needing a PostgreSQL connection
LocalBackupInfo(server, backup_id="fake_backup_id")

0 comments on commit dde2d12

Please sign in to comment.