Skip to content

[Filestore] Fix data corruption on unaligned requests exceeding file size #4630

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

[Filestore] Fix data corruption on unaligned requests exceeding file size #4630

wants to merge 9 commits into from

Conversation

@antonmyagkov
Copy link
Collaborator

@antonmyagkov antonmyagkov commented Nov 7, 2025 •
edited
Loading

issue: #4695

Mark the last block that exceeds the file size as deleted.

STR:

  1. Write to the file using unaligned requests to populate the
    fresh blocks and fresh bytes lists. A 16 KB unaligned write that
    extends beyond the current file size produces one fresh-bytes record
    and four fresh blocks, since the current implementation pads the last
    block with zeros to align it.
  2. Flush the fresh blocks.
  3. Overwrite the data at the end of the file with another 16 KB
    unaligned request. This time the write does not extend the file size
    and results in two fresh-bytes records and three fresh blocks.
  4. Flush the fresh blocks again.
  5. Flush the fresh bytes.
  6. Read and validate the expected data from the tail of the file.

@antonmyagkov antonmyagkov added the filestore Add this label to run only cloud/filestore build and tests on PR label Nov 7, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Nov 7, 2025

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit e8846bb.

TESTS PASSED ERRORS FAILED FAILED BUILD SKIPPED MUTED?
3151 3151 0 0 0 0 0

@antonmyagkov antonmyagkov added the recheck Add this label to relaunch checks, it will be automatically removed label Nov 7, 2025
@github-actions github-actions bot removed the recheck Add this label to relaunch checks, it will be automatically removed label Nov 7, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Nov 7, 2025 •
edited
Loading

Note

This is an automated comment that will be appended during run.

🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit e8846bb.

TESTS PASSED ERRORS FAILED FAILED BUILD SKIPPED MUTED?
3151 3148 0 3 0 0 0

🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit e8846bb.

TESTS PASSED ERRORS FAILED FAILED BUILD SKIPPED MUTED?
6 5 0 1 0 0 0

🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit e8846bb.

TESTS PASSED ERRORS FAILED FAILED BUILD SKIPPED MUTED?
2 1 0 1 0 0 0

@antonmyagkov antonmyagkov changed the title fio: set offset 256KB-1 [Filestore] Fix data corruption on unaligned requests exceeding file size Nov 11, 2025
@antonmyagkov antonmyagkov force-pushed the users/myagkov/fio-corruption branch from 123d554 to f517195 Compare November 11, 2025 20:26
@github-actions
Copy link
Contributor

github-actions bot commented Nov 11, 2025 •
edited
Loading

Note

This is an automated comment that will be appended during run.

🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit f517195.

TESTS PASSED ERRORS FAILED FAILED BUILD SKIPPED MUTED?
3151 3150 0 1 0 0 0

🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit f517195.

TESTS PASSED ERRORS FAILED FAILED BUILD SKIPPED MUTED?
2 1 0 1 0 0 0

🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit f517195.

TESTS PASSED ERRORS FAILED FAILED BUILD SKIPPED MUTED?
2 1 0 1 0 0 0

@github-actions
Copy link
Contributor

github-actions bot commented Nov 12, 2025

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit 964e574.

TESTS PASSED ERRORS FAILED FAILED BUILD SKIPPED MUTED?
3151 3151 0 0 0 0 0

@antonmyagkov antonmyagkov added the large-tests Launch large tests for PR label Nov 12, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Nov 12, 2025

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit 964e574.

TESTS PASSED ERRORS FAILED FAILED BUILD SKIPPED MUTED?
3185 3185 0 0 0 0 0

@antonmyagkov antonmyagkov marked this pull request as ready for review November 13, 2025 08:56
@github-actions
Copy link
Contributor

github-actions bot commented Nov 25, 2025

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit 5c95d20.

TESTS PASSED ERRORS FAILED FAILED BUILD SKIPPED MUTED?
3186 3186 0 0 0 0 0

SvartMetal
SvartMetal reviewed Nov 25, 2025
View reviewed changes
SvartMetal
SvartMetal reviewed Nov 25, 2025
View reviewed changes
@github-actions
Copy link
Contributor

github-actions bot commented Nov 25, 2025

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit bc91667.

TESTS PASSED ERRORS FAILED FAILED BUILD SKIPPED MUTED?
3186 3186 0 0 0 0 0

SvartMetal
SvartMetal previously approved these changes Nov 25, 2025
View reviewed changes
debnatkh
debnatkh reviewed Nov 26, 2025
View reviewed changes
cloud/filestore/libs/storage/tablet/tablet_ut_data.cpp
ui64 handle = CreateHandle(tablet, id);

// the numbers were adjusted to have the tail block in
// the same blob as other blocks from fresh bytes list
Copy link
Collaborator

@debnatkh debnatkh Nov 26, 2025

Choose a reason for hiding this comment

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

Лучше явно убедиться, что rangeId / BlockGroupSize отличаются

Copy link
Collaborator Author

@antonmyagkov antonmyagkov Nov 27, 2025

Choose a reason for hiding this comment

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

Добавил проверку что блок хвоста совпадает с одним из блоков для которого вызывается FlushBytes

@github-actions
Copy link
Contributor

github-actions bot commented Nov 27, 2025

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit bc090dd.

TESTS PASSED ERRORS FAILED FAILED BUILD SKIPPED MUTED?
3187 3187 0 0 0 0 0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@debnatkh debnatkh Awaiting requested review from debnatkh

@SvartMetal SvartMetal Awaiting requested review from SvartMetal

At least 2 approving reviews are required to merge this pull request.

Assignees

No one assigned

Labels

filestore Add this label to run only cloud/filestore build and tests on PR large-tests Launch large tests for PR

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@antonmyagkov @SvartMetal @debnatkh