Skip to content
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

Provide atomicity of state dir copying #7055

Merged

Conversation

kozlovsky
Copy link
Contributor

This PR fixes the source of #7002 by providing atomicity for state directory copying. To achieve that, it first copies the state into a temporary dir and then renames it to a correct name after the copying process is finished.

Also, it adds logging to the version manager, making it possible to see upgrade-related events in error reports.

@kozlovsky kozlovsky marked this pull request as ready for review September 20, 2022 04:03
@kozlovsky kozlovsky requested review from a team, drew2a and xoriole and removed request for a team September 20, 2022 04:03
@@ -103,6 +108,9 @@ def __repr__(self):
def get_directory(self):
return self.root_state_dir / ('%d.%d' % self.major_minor)

def get_tmp_copy_directory(self):
return self.root_state_dir / ('%d.%d_tmp_copy' % self.major_minor)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: Maybe it is a bit safer to generate some random postfix for this temporary directory

Suggested change
return self.root_state_dir / ('%d.%d_tmp_copy' % self.major_minor)
major = self.major_minor[0]
minor = self.major_minor[1]
temporary_dir_name = f'{major}.{minor}_tmp_copy_{random.randrange(100)}'
return self.root_state_dir / temporary_dir_name

@kozlovsky kozlovsky changed the base branch from main to release/7.12 September 20, 2022 09:54
@kozlovsky kozlovsky merged commit 4d953de into Tribler:release/7.12 Sep 20, 2022
@kozlovsky kozlovsky deleted the fix/copy_state_dir_atomicity branch September 20, 2022 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants