-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Parallel FREEZE TABLE #164
Changes from 12 commits
4c3acbf
faecbed
8807c86
1e86b09
6b7182b
c45b386
ec20451
8b795ae
4f8bfec
7554870
06cb097
95bc77b
0214f1e
6933163
d0a25c9
827a66d
93da739
b7a4358
6d1076a
c70ad1f
190539c
59fdf82
2f12cb7
87f9d30
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,6 +24,7 @@ | |
from ch_backup.exceptions import ClickhouseBackupError | ||
from ch_backup.util import ( | ||
chown_dir_contents, | ||
chown_file, | ||
escape, | ||
list_dir_files, | ||
retry, | ||
|
@@ -474,15 +475,27 @@ def system_unfreeze(self, backup_name: str) -> None: | |
query_sql = SYSTEM_UNFREEZE_SQL.format(backup_name=backup_name) | ||
self._ch_client.query(query_sql, timeout=self._unfreeze_timeout) | ||
|
||
def remove_freezed_data(self) -> None: | ||
def remove_freezed_data( | ||
self, backup_name: Optional[str] = None, table: Optional[Table] = None | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's introduce a new object with backup_name and table as members. Or add checking and throwing an exception when only one from the pair is provided. |
||
) -> None: | ||
""" | ||
Remove all freezed partitions from all local disks. | ||
""" | ||
for disk in self._disks.values(): | ||
if disk.type == "local": | ||
shadow_path = os.path.join(disk.path, "shadow") | ||
logging.debug("Removing shadow data: {}", shadow_path) | ||
self._remove_shadow_data(shadow_path) | ||
if backup_name and table: | ||
for table_data_path, disk in table.paths_with_disks: | ||
if disk.type == "local": | ||
table_relative_path = os.path.relpath(table_data_path, disk.path) | ||
shadow_path = os.path.join( | ||
disk.path, "shadow", backup_name, table_relative_path | ||
) | ||
logging.debug("Removing shadow data: {}", shadow_path) | ||
self._remove_shadow_data(shadow_path) | ||
else: | ||
for disk in self._disks.values(): | ||
if disk.type == "local": | ||
shadow_path = os.path.join(disk.path, "shadow") | ||
logging.debug("Removing shadow data: {}", shadow_path) | ||
self._remove_shadow_data(shadow_path) | ||
|
||
def remove_freezed_part(self, part: FrozenPart) -> None: | ||
""" | ||
|
@@ -776,6 +789,26 @@ def chown_dir(self, dir_path: str) -> None: | |
need_recursion, | ||
) | ||
|
||
def create_shadow_increment(self) -> None: | ||
""" | ||
Create shadow/increment.txt to fix race condition with parallel freeze. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there some issue or comment in the code of CH about this to mention here ? |
||
Must be used before freezing more than one table at once. | ||
""" | ||
default_shadow_path = Path(self._root_data_path) / "shadow" | ||
increment_path = default_shadow_path / "increment.txt" | ||
if os.path.exists(increment_path): | ||
return | ||
if not os.path.exists(default_shadow_path): | ||
os.mkdir(default_shadow_path) | ||
self.chown_dir(str(default_shadow_path)) | ||
with open(increment_path, "w", encoding="utf-8") as file: | ||
file.write("0") | ||
chown_file( | ||
self._ch_ctl_config["user"], | ||
self._ch_ctl_config["group"], | ||
str(increment_path), | ||
) | ||
|
||
@retry(OSError) | ||
def _remove_shadow_data(self, path: str) -> None: | ||
if path.find("/shadow") == -1: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
Feature: Parallel freeze | ||
|
||
Background: | ||
Given default configuration | ||
And a working s3 | ||
And a working zookeeper on zookeeper01 | ||
And a working clickhouse on clickhouse01 | ||
And a working clickhouse on clickhouse02 | ||
And clickhouse on clickhouse01 has test schema with 5 databases and 10 tables | ||
And clickhouse01 has test clickhouse data test1 with 5 databases, 10 tables, 100 rows and 5 partitions | ||
|
||
Scenario: Create backup with single freeze worker | ||
Given ch-backup configuration on clickhouse01 | ||
""" | ||
multiprocessing: | ||
freeze_workers: 1 | ||
""" | ||
When we create clickhouse01 clickhouse backup | ||
Then we got the following backups on clickhouse01 | ||
| num | state | data_count | link_count | title | | ||
| 0 | created | 250 | 0 | shared | | ||
When we restore clickhouse backup #0 to clickhouse02 | ||
Then we got same clickhouse data at clickhouse01 clickhouse02 | ||
|
||
Scenario: Create backup with default number of freeze workers | ||
When we create clickhouse01 clickhouse backup | ||
Then we got the following backups on clickhouse01 | ||
| num | state | data_count | link_count | title | | ||
| 0 | created | 250 | 0 | shared | | ||
When we restore clickhouse backup #0 to clickhouse02 | ||
Then we got same clickhouse data at clickhouse01 clickhouse02 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It makes sense use
get()
only when a default is expected. The error of absent key for[]
is more readable.