diff --git a/ch_backup/logic/table.py b/ch_backup/logic/table.py index 1db5101d..91603a5b 100644 --- a/ch_backup/logic/table.py +++ b/ch_backup/logic/table.py @@ -2,7 +2,6 @@ Clickhouse backup logic for tables """ import os -import shutil from collections import deque from dataclasses import dataclass from functools import partial @@ -323,42 +322,20 @@ def _backup_table( ) return - # Sometimes while ALTER FREEZing a table we may get an error that a directory for - # part already exists. This clearly is a clickhouse error but to mitigate it on backup - # side we can just clear this directory as it doesn't contain actual data, just hardlinks - retry_on_existing_dir: int = context.config.get("retry_on_existing_dir", 0) - dir_exists_prefix: str = "Code: 84. DB::Exception: Directory " - # Freeze only MergeTree tables if not schema_only and is_merge_tree(table.engine): - for i in range(retry_on_existing_dir + 1): - try: - context.ch_ctl.freeze_table(backup_name, table) - break - except ClickhouseError as e: - if not context.ch_ctl.does_table_exist(table.database, table.name): - logging.warning( - 'Table "{}"."{}" was removed by a user during backup', - table.database, - table.name, - ) - return - - if retry_on_existing_dir > 0 and i == retry_on_existing_dir: - raise ClickhouseError( - f"All {retry_on_existing_dir} retries failed" - ) from e - if retry_on_existing_dir > 0 and str(e).startswith( - dir_exists_prefix - ): - existing_dir = str(e)[len(dir_exists_prefix) :].split( - " ", maxsplit=1 - )[0] - shutil.rmtree(existing_dir) - logging.debug(f"Removed existing dir {existing_dir}, retrying") - continue + try: + context.ch_ctl.freeze_table(backup_name, table) + except ClickhouseError: + if context.ch_ctl.does_table_exist(table.database, table.name): + raise - raise e + logging.warning( + 'Table "{}"."{}" was removed by a user during backup', + table.database, + table.name, + ) + return # Check if table metadata was updated new_mtime = self._get_mtime(table.metadata_path) diff --git a/tests/integration/features/single_table.feature b/tests/integration/features/single_table.feature index 0534c4f2..b7eca0b5 100644 --- a/tests/integration/features/single_table.feature +++ b/tests/integration/features/single_table.feature @@ -24,43 +24,3 @@ Feature: Backup of single database table tables: - test_db_01.test_table_01 """ - - Scenario: Create backup with existing paths in backup directory - Given clickhouse01 has test clickhouse data test1 - When we execute query on clickhouse01 - """ - ALTER TABLE test_db_01.test_table_01 FREEZE - """ - When we try to execute command on clickhouse01 - """ - find /var/lib/clickhouse/shadow/1/store/*/*/* -print | head -n 1 > part_dir - """ - When we execute query on clickhouse01 - """ - # Strange but you can't UNFREEZE without specifying name - ALTER TABLE test_db_01.test_table_01 UNFREEZE with name '1' - """ - When we try to execute command on clickhouse01 - """ - part_dir=$(cat part_dir) - mkdir -p $part_dir - echo "test" > $part_dir/doo - chown -R clickhouse:clickhouse $part_dir - """ - When we can't create clickhouse01 clickhouse backup with exception - """ - tables: - - test_db_01.test_table_01 - name: '1' - """ - When we update ch-backup configuration on clickhouse01 - """ - backup: - retry_on_existing_dir: 1 - """ - When we create clickhouse01 clickhouse backup - """ - tables: - - test_db_01.test_table_01 - name: '1' - """ diff --git a/tests/integration/steps/ch_backup.py b/tests/integration/steps/ch_backup.py index d5c8f49c..71c01a8a 100644 --- a/tests/integration/steps/ch_backup.py +++ b/tests/integration/steps/ch_backup.py @@ -3,7 +3,6 @@ """ import json -import pytest from behave import given, then, when from hamcrest import ( any_of, @@ -19,7 +18,6 @@ @given("ch-backup configuration on {node:w}") -@when("we update ch-backup configuration on {node:w}") def step_update_ch_backup_config(context, node): conf = get_step_data(context) BackupManager(context, node).update_config(conf) @@ -47,13 +45,6 @@ def step_cannot_create_backup(context, node): ) -@when("we can't create {node:w} clickhouse backup with exception") -def step_cannot_create_backup_exc(context, node): - options = get_step_data(context) - with pytest.raises(Exception): - BackupManager(context, node).backup(**options) - - @given("metadata of {node:w} backup #{backup_id:d} was adjusted with") @when("metadata of {node:w} backup #{backup_id:d} was adjusted with") @given('metadata of {node:w} backup "{backup_id}" was adjusted with')