diff --git a/NEWS b/NEWS index bee04626..1d5edc4c 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,9 @@ 2.1.6.dev0 + * #1267: Support multiple borgmatic instances run in parallel—as long as they're operating on + different Borg repositories. This required an internal change to the runtime directory path, + which may cause a one-time performance hit (Borg cache misses) for Borg 1.x users of database and + filesystem hooks. See the documentation for more information: + https://torsion.org/borgmatic/how-to/make-per-application-backups/#parallelism * #1300: Fix the "source_directories_must_exist" option to support source directories relative to a "working_directory". * #1301: Expand the "patterns_from" and "exclude_from" options to support paths containing tildes diff --git a/borgmatic/actions/check.py b/borgmatic/actions/check.py index 7967aaf3..33b58cc7 100644 --- a/borgmatic/actions/check.py +++ b/borgmatic/actions/check.py @@ -20,6 +20,7 @@ import borgmatic.borg.environment import borgmatic.borg.extract import borgmatic.borg.list import borgmatic.borg.pattern +import borgmatic.borg.repo_info import borgmatic.borg.repo_list import borgmatic.config.paths import borgmatic.execute @@ -788,15 +789,7 @@ def run_check( ''' logger.info('Running consistency checks') - repository_id = borgmatic.borg.check.get_repository_id( - repository['path'], - config, - local_borg_version, - global_arguments, - local_path=local_path, - remote_path=remote_path, - ) - upgrade_check_times(config, repository_id) + upgrade_check_times(config, repository['id']) configured_checks = parse_checks(config, check_arguments.only_checks) archive_filter_flags = borgmatic.borg.check.make_archive_filter_flags( local_borg_version, @@ -807,7 +800,7 @@ def run_check( archives_check_id = make_archives_check_id(archive_filter_flags) checks = filter_checks_on_frequency( config, - repository_id, + repository['id'], configured_checks, check_arguments.force, archives_check_id, @@ -827,7 +820,7 @@ def run_check( remote_path=remote_path, ) for check in borg_specific_checks: - write_check_time(make_check_time_path(config, repository_id, check, archives_check_id)) + write_check_time(make_check_time_path(config, repository['id'], check, archives_check_id)) if 'extract' in checks: logger.info('Running extract check') @@ -840,11 +833,11 @@ def run_check( local_path, remote_path, ) - write_check_time(make_check_time_path(config, repository_id, 'extract')) + write_check_time(make_check_time_path(config, repository['id'], 'extract')) if 'spot' in checks: logger.info('Running spot check') - with borgmatic.config.paths.Runtime_directory(config) as borgmatic_runtime_directory: + with borgmatic.config.paths.Runtime_directory(config, repository['id']) as borgmatic_runtime_directory: spot_check( repository, config, @@ -855,4 +848,4 @@ def run_check( borgmatic_runtime_directory, ) - write_check_time(make_check_time_path(config, repository_id, 'spot')) + write_check_time(make_check_time_path(config, repository['id'], 'spot')) diff --git a/borgmatic/actions/config/bootstrap.py b/borgmatic/actions/config/bootstrap.py index a561c7b7..b00d96c3 100644 --- a/borgmatic/actions/config/bootstrap.py +++ b/borgmatic/actions/config/bootstrap.py @@ -109,8 +109,16 @@ def run_bootstrap(bootstrap_arguments, global_arguments, local_borg_version): local_path=bootstrap_arguments.local_path, remote_path=bootstrap_arguments.remote_path, ) + repository_id = borgmatic.borg.repo_info.get_repository_id( + bootstrap_arguments.repository, + config, + local_borg_version, + global_arguments, + local_path=bootstrap_arguments.local_path, + remote_path=bootstrap_arguments.remote_path, + ) - with borgmatic.config.paths.Runtime_directory(config) as borgmatic_runtime_directory: + with borgmatic.config.paths.Runtime_directory(config, repository_id) as borgmatic_runtime_directory: manifest_config_paths = load_config_paths_from_archive( bootstrap_arguments.repository, archive_name, diff --git a/borgmatic/actions/create.py b/borgmatic/actions/create.py index e307ffd7..5a3453e8 100644 --- a/borgmatic/actions/create.py +++ b/borgmatic/actions/create.py @@ -43,7 +43,7 @@ def run_create( logger.info(f'Creating archive{dry_run_label}') working_directory = borgmatic.config.paths.get_working_directory(config) - with borgmatic.config.paths.Runtime_directory(config) as borgmatic_runtime_directory: + with borgmatic.config.paths.Runtime_directory(config, repository['id']) as borgmatic_runtime_directory: patterns = pattern.process_patterns( pattern.collect_patterns(config, working_directory), config, diff --git a/borgmatic/actions/restore.py b/borgmatic/actions/restore.py index 3f38b843..c31669e0 100644 --- a/borgmatic/actions/restore.py +++ b/borgmatic/actions/restore.py @@ -537,7 +537,7 @@ def run_restore( logger.info(f'Restoring data sources from archive {restore_arguments.archive}') working_directory = borgmatic.config.paths.get_working_directory(config) - with borgmatic.config.paths.Runtime_directory(config) as borgmatic_runtime_directory: + with borgmatic.config.paths.Runtime_directory(config, repository['id']) as borgmatic_runtime_directory: patterns = borgmatic.actions.pattern.process_patterns( borgmatic.actions.pattern.collect_patterns(config, working_directory), config, diff --git a/borgmatic/borg/check.py b/borgmatic/borg/check.py index 64ea23cd..7130812c 100644 --- a/borgmatic/borg/check.py +++ b/borgmatic/borg/check.py @@ -1,10 +1,8 @@ -import argparse -import json import logging import shlex import borgmatic.config.paths -from borgmatic.borg import environment, feature, flags, repo_info +from borgmatic.borg import environment, feature, flags from borgmatic.execute import DO_NOT_CAPTURE, execute_command logger = logging.getLogger(__name__) @@ -82,36 +80,6 @@ def make_check_name_flags(checks, archive_filter_flags): ) -def get_repository_id( - repository_path, - config, - local_borg_version, - global_arguments, - local_path, - remote_path, -): - ''' - Given a local or remote repository path, a configuration dict, the local Borg version, global - arguments, and local/remote commands to run, return the corresponding Borg repository ID. - - Raise ValueError if the Borg repository ID cannot be determined. - ''' - try: - return json.loads( - repo_info.display_repository_info( - repository_path, - config, - local_borg_version, - argparse.Namespace(json=True), - global_arguments, - local_path, - remote_path, - ), - )['repository']['id'] - except (json.JSONDecodeError, KeyError): - raise ValueError(f'Cannot determine Borg repository ID for {repository_path}') - - def check_archives( repository_path, config, diff --git a/borgmatic/borg/repo_info.py b/borgmatic/borg/repo_info.py index 9924aa8d..dacf4943 100644 --- a/borgmatic/borg/repo_info.py +++ b/borgmatic/borg/repo_info.py @@ -1,3 +1,5 @@ +import argparse +import json import logging import shlex @@ -81,3 +83,33 @@ def display_repository_info( ) return None + + +def get_repository_id( + repository_path, + config, + local_borg_version, + global_arguments, + local_path, + remote_path, +): + ''' + Given a local or remote repository path, a configuration dict, the local Borg version, global + arguments, and local/remote commands to run, return the corresponding Borg repository ID. + + Raise ValueError if the Borg repository ID cannot be determined. + ''' + try: + return json.loads( + display_repository_info( + repository_path, + config, + local_borg_version, + argparse.Namespace(json=True), + global_arguments, + local_path, + remote_path, + ), + )['repository']['id'] + except (json.JSONDecodeError, KeyError): + raise ValueError(f'Cannot determine Borg repository ID for {repository_path}') diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index 83c77829..ab5812b8 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -381,6 +381,15 @@ def run_actions( # noqa: PLR0912, PLR0915 logger.debug('Skipping actions because the requested --repository does not match') return + repository['id'] = borgmatic.borg.repo_info.get_repository_id( + repository['path'], + config, + local_borg_version, + global_arguments, + local_path=local_path, + remote_path=remote_path, + ) + with borgmatic.hooks.command.Before_after_hooks( command_hooks=config.get('commands'), before_after='repository', diff --git a/borgmatic/config/paths.py b/borgmatic/config/paths.py index 1788d3e2..24429c84 100644 --- a/borgmatic/config/paths.py +++ b/borgmatic/config/paths.py @@ -2,6 +2,7 @@ import contextlib import logging import os import tempfile +import shutil from enum import Enum logger = logging.getLogger(__name__) @@ -84,6 +85,24 @@ def replace_temporary_subdirectory_with_glob( ) +class Fixed_name_temporary_directory: + ''' + A class whose instances can stand-in for tempfile.TemporaryDirectory's, except the temporary + filename path is fixed rather than randomly generated. + ''' + def __init__(self, path): + ''' + Given a temporary directory path, save it off for later. + ''' + self.name = path + + def cleanup(self): + ''' + Remove the temporary directory path. + ''' + shutil.rmtree(self.name) + + class Runtime_directory: ''' A Python context manager for creating and cleaning up the borgmatic runtime directory used for @@ -98,16 +117,15 @@ class Runtime_directory: automatically gets cleaned up as necessary. ''' - def __init__(self, config): + def __init__(self, config, repository_id): ''' - Given a configuration dict determine the borgmatic runtime directory, creating a secure, - temporary directory within it if necessary. Defaults to $XDG_RUNTIME_DIR/./borgmatic or - $RUNTIME_DIRECTORY/./borgmatic or $TMPDIR/borgmatic-[random]/./borgmatic or - $TEMP/borgmatic-[random]/./borgmatic or /tmp/borgmatic-[random]/./borgmatic where "[random]" - is a randomly generated string intended to avoid path collisions. - - If XDG_RUNTIME_DIR or RUNTIME_DIRECTORY is set and already ends in "/borgmatic", then don't - tack on a second "/borgmatic" path component. + Given a configuration dict and the Borg ID for a repository, determine the borgmatic runtime + directory, creating a secure, temporary directory within it if necessary. Defaults to + $XDG_RUNTIME_DIR/[repository_id]/./borgmatic or $RUNTIME_DIRECTORY/[repository_id]/./borgmatic or + $TMPDIR/borgmatic-[random]/./borgmatic or $TEMP/borgmatic-[random]/./borgmatic or + /tmp/borgmatic-[random]/./borgmatic where "[random]" is a randomly generated string and + "[repository_id]" is the Borg repository ID. Both are intended to avoid path collisions, and + the random string helps avoid temporary file attacks. The "/./" is taking advantage of a Borg feature such that the part of the path before the "/./" does not get stored in the file path within an archive. That way, the path of the runtime @@ -125,7 +143,8 @@ class Runtime_directory: if not runtime_directory.startswith(os.path.sep): raise ValueError('The runtime directory must be an absolute path') - self.temporary_directory = None + runtime_directory = os.path.join(runtime_directory, f'borgmatic-{repository_id}') + self.temporary_directory = Fixed_name_temporary_directory(runtime_directory) else: base_directory = ( os.environ.get('TMPDIR') or os.environ.get('TEMP') or '/tmp' # noqa: S108 @@ -141,11 +160,9 @@ class Runtime_directory: ) runtime_directory = self.temporary_directory.name - (base_path, final_directory) = os.path.split(runtime_directory.rstrip(os.path.sep)) - self.runtime_path = expand_user_in_path( os.path.join( - base_path if final_directory == 'borgmatic' else runtime_directory, + runtime_directory, '.', # Borg 1.4+ "slashdot" hack. 'borgmatic', ), @@ -162,14 +179,13 @@ class Runtime_directory: def __exit__(self, exception_type, exception, traceback): ''' - Delete any temporary directory that was created as part of initialization. + Delete the temporary directory that was created as part of initialization. ''' - if self.temporary_directory: - # The cleanup() call errors if, for instance, there's still a - # mounted filesystem within the temporary directory. There's - # nothing we can do about that here, so swallow the error. - with contextlib.suppress(OSError): - self.temporary_directory.cleanup() + # The cleanup() call errors if, for instance, there's still a + # mounted filesystem within the temporary directory. There's + # nothing we can do about that here, so swallow the error. + with contextlib.suppress(OSError): + self.temporary_directory.cleanup() def make_runtime_directory_glob(borgmatic_runtime_directory): diff --git a/docs/how-to/make-per-application-backups.md b/docs/how-to/make-per-application-backups.md index 7e196e24..72767a03 100644 --- a/docs/how-to/make-per-application-backups.md +++ b/docs/how-to/make-per-application-backups.md @@ -58,19 +58,25 @@ choice](https://torsion.org/borgmatic/how-to/set-up-backups/#autopilot), each entry using borgmatic's `--config` flag instead of relying on `/etc/borgmatic.d`. + -## Limitations +## Parallelism -borgmatic does not currently support its own parallelism—being run multiple -times on the same machine simultaneously. In particular, many of the [data -source -hooks](https://torsion.org/borgmatic/reference/configuration/data-sources/) rely -on global borgmatic runtime files which can't be shared across processes, and -therefore multiple borgmatic instances on the same machine would interfere with -each other. +New in version 2.1.6 borgmatic +supports multiple borgmatic instances run on the same machine in parallel—as +long as they're operating on different Borg repositories. -A single borgmatic instance also doesn't currently support running multiple Borg -instances in parallel on the same machine. +However, note that a single borgmatic instance doesn't currently support running +multiple Borg instances in parallel on the same machine. + + +Prior to version 2.1.6 borgmatic +did not support parallel borgmatic runs on the same machine simultaneously. In +particular, many of the [data source +hooks](https://torsion.org/borgmatic/reference/configuration/data-sources/) +relied on global borgmatic runtime files which couldn't be shared across +processes, and therefore multiple borgmatic instances on the same machine +interfered with each other. diff --git a/docs/reference/configuration/runtime-directory.md b/docs/reference/configuration/runtime-directory.md index 8eec495c..59b5429a 100644 --- a/docs/reference/configuration/runtime-directory.md +++ b/docs/reference/configuration/runtime-directory.md @@ -29,11 +29,18 @@ probes the following values: You can see the runtime directory path that borgmatic selects by running with `--verbosity 2` and looking for `Using runtime directory` in the output. -Regardless of the runtime directory selected, borgmatic stores its files -within a `borgmatic` subdirectory of the runtime directory. Additionally, in -the case of `TMPDIR`, `TEMP`, and the hard-coded `/tmp`, borgmatic creates a -randomly named subdirectory in an effort to reduce path collisions in shared -system temporary directories. +Regardless of the runtime directory selected, borgmatic stores its files within +a `borgmatic` subdirectory of the runtime directory. Additionally, in the case +of `TMPDIR`, `TEMP`, and the hard-coded `/tmp`, borgmatic creates a randomly +named subdirectory in an effort to reduce path collisions and temporary file +attacks in shared system temporary directories. + +New in version 2.1.6 When +constructing the runtime directory, borgmatic now creates a subdirectory named +after the Borg ID of the current repository. This means that borgmatic supports +multiple borgmatic instances run [in +parallel](https://torsion.org/borgmatic/how-to/make-per-application-backups/#parallelism)—as +long as they're operating on different Borg repositories. Prior to version 1.9.0 borgmatic created temporary streaming database dumps within the `~/.borgmatic` diff --git a/tests/unit/actions/test_check.py b/tests/unit/actions/test_check.py index 7d0d2037..b4a07504 100644 --- a/tests/unit/actions/test_check.py +++ b/tests/unit/actions/test_check.py @@ -1838,7 +1838,9 @@ def test_spot_check_without_any_source_paths_errors(): def test_run_check_checks_archives_for_configured_repository(): flexmock(module.logger).answer = lambda message: None - flexmock(module.borgmatic.borg.check).should_receive('get_repository_id').and_return(flexmock()) + flexmock(module.borgmatic.borg.repo_info).should_receive('get_repository_id').and_return( + flexmock() + ) flexmock(module).should_receive('upgrade_check_times') flexmock(module).should_receive('parse_checks') flexmock(module.borgmatic.borg.check).should_receive('make_archive_filter_flags').and_return(()) @@ -1873,7 +1875,9 @@ def test_run_check_checks_archives_for_configured_repository(): def test_run_check_runs_configured_extract_check(): flexmock(module.logger).answer = lambda message: None - flexmock(module.borgmatic.borg.check).should_receive('get_repository_id').and_return(flexmock()) + flexmock(module.borgmatic.borg.repo_info).should_receive('get_repository_id').and_return( + flexmock() + ) flexmock(module).should_receive('upgrade_check_times') flexmock(module).should_receive('parse_checks') flexmock(module.borgmatic.borg.check).should_receive('make_archive_filter_flags').and_return(()) @@ -1906,7 +1910,9 @@ def test_run_check_runs_configured_extract_check(): def test_run_check_runs_configured_spot_check(): flexmock(module.logger).answer = lambda message: None - flexmock(module.borgmatic.borg.check).should_receive('get_repository_id').and_return(flexmock()) + flexmock(module.borgmatic.borg.repo_info).should_receive('get_repository_id').and_return( + flexmock() + ) flexmock(module).should_receive('upgrade_check_times') flexmock(module).should_receive('parse_checks') flexmock(module.borgmatic.borg.check).should_receive('make_archive_filter_flags').and_return(()) @@ -1942,7 +1948,9 @@ def test_run_check_runs_configured_spot_check(): def test_run_check_without_checks_runs_nothing_except_hooks(): flexmock(module.logger).answer = lambda message: None - flexmock(module.borgmatic.borg.check).should_receive('get_repository_id').and_return(flexmock()) + flexmock(module.borgmatic.borg.repo_info).should_receive('get_repository_id').and_return( + flexmock() + ) flexmock(module).should_receive('upgrade_check_times') flexmock(module).should_receive('parse_checks') flexmock(module.borgmatic.borg.check).should_receive('make_archive_filter_flags').and_return(()) diff --git a/tests/unit/borg/test_check.py b/tests/unit/borg/test_check.py index d68c89e0..e82be157 100644 --- a/tests/unit/borg/test_check.py +++ b/tests/unit/borg/test_check.py @@ -272,54 +272,6 @@ def test_make_check_name_flags_with_extract_omits_extract_flag(): assert flags == () -def test_get_repository_id_with_valid_json_does_not_raise(): - config = {} - flexmock(module.repo_info).should_receive('display_repository_info').and_return( - '{"repository": {"id": "repo"}}', - ) - - assert module.get_repository_id( - repository_path='repo', - config=config, - local_borg_version='1.2.3', - global_arguments=flexmock(), - local_path='borg', - remote_path=None, - ) - - -def test_get_repository_id_with_json_error_raises(): - config = {} - flexmock(module.repo_info).should_receive('display_repository_info').and_return( - '{"unexpected": {"id": "repo"}}', - ) - - with pytest.raises(ValueError): - module.get_repository_id( - repository_path='repo', - config=config, - local_borg_version='1.2.3', - global_arguments=flexmock(), - local_path='borg', - remote_path=None, - ) - - -def test_get_repository_id_with_missing_json_keys_raises(): - config = {} - flexmock(module.repo_info).should_receive('display_repository_info').and_return('{invalid JSON') - - with pytest.raises(ValueError): - module.get_repository_id( - repository_path='repo', - config=config, - local_borg_version='1.2.3', - global_arguments=flexmock(), - local_path='borg', - remote_path=None, - ) - - def test_check_archives_with_progress_passes_through_to_borg(): config = {'progress': True} flexmock(module).should_receive('make_check_name_flags').with_args( diff --git a/tests/unit/borg/test_repo_info.py b/tests/unit/borg/test_repo_info.py index a85837ff..aeb889e3 100644 --- a/tests/unit/borg/test_repo_info.py +++ b/tests/unit/borg/test_repo_info.py @@ -1,5 +1,6 @@ import logging +import pytest from flexmock import flexmock from borgmatic.borg import repo_info as module @@ -612,3 +613,51 @@ def test_display_repository_info_calls_borg_with_working_directory(): repo_info_arguments=flexmock(json=False), global_arguments=flexmock(), ) + + +def test_get_repository_id_with_valid_json_does_not_raise(): + config = {} + flexmock(module).should_receive('display_repository_info').and_return( + '{"repository": {"id": "repo"}}', + ) + + assert module.get_repository_id( + repository_path='repo', + config=config, + local_borg_version='1.2.3', + global_arguments=flexmock(), + local_path='borg', + remote_path=None, + ) + + +def test_get_repository_id_with_json_error_raises(): + config = {} + flexmock(module).should_receive('display_repository_info').and_return( + '{"unexpected": {"id": "repo"}}', + ) + + with pytest.raises(ValueError): + module.get_repository_id( + repository_path='repo', + config=config, + local_borg_version='1.2.3', + global_arguments=flexmock(), + local_path='borg', + remote_path=None, + ) + + +def test_get_repository_id_with_missing_json_keys_raises(): + config = {} + flexmock(module).should_receive('display_repository_info').and_return('{invalid JSON') + + with pytest.raises(ValueError): + module.get_repository_id( + repository_path='repo', + config=config, + local_borg_version='1.2.3', + global_arguments=flexmock(), + local_path='borg', + remote_path=None, + )