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

Store read-only flag into metadata file for DiskS3 #17227

Merged
merged 2 commits into from
Nov 23, 2020

Conversation

Jokser
Copy link
Contributor

@Jokser Jokser commented Nov 20, 2020

I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en

Changelog category (leave one):

  • Bug Fix

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Fixed possible not-working mutations for parts stored on S3 disk.

Detailed description / Documentation draft:
When we perform local backup of a data part (e.g. doing mutations, cloning) it does the following things with each file of the part:

  1. Mark file as read-only
  2. Create hard-link from that file to destination path

Marking file as read-only in DiskS3 just sets FS read-only flag for metadata file.
When we create hard-link from that file it modifies metadata file incrementing number of references. This is actually file modification operation and it breaks, because metadata file was marked as read-only right before:
https://gist.github.com/Jokser/d282052507b4592dca78b9c61d1295ca

We shouldn't mark file as read-only on FS in that case, instead we should store read-only inside metadata file and check it if we trying to re-write this file.

@robot-clickhouse robot-clickhouse added the pr-bugfix Pull request with bugfix, not backported by default label Nov 20, 2020
@alexey-milovidov alexey-milovidov self-assigned this Nov 23, 2020
@Jokser
Copy link
Contributor Author

Jokser commented Nov 23, 2020

Failed test_storage_s3/test.py::test_custom_auth_headers I'm fixing there
#17299

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-bugfix Pull request with bugfix, not backported by default
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants