Skip to content

Fix TEvPatch behaviour #1469

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

Merged
merged 1 commit into from
Jan 31, 2024

Conversation

alexvru
Copy link
Collaborator

@alexvru alexvru commented Jan 31, 2024

Changelog entry

Fix bug in TEvPatch event processing pipeline leading to probable corruption of original blob.

Changelog category

  • Bugfix

Additional information

TEvPatch processing sometimes involves inter-VDisk operation, which uses TSkeletonVPatchActor. This actor reads existing blobs through TEvVGet queries and operates on data received in TEvVGetResult responses. Previously they were delivered as "bytes" protobuf fields, then the logic was changed to use TRope payloads. Also, there was a change that switched internally-stored responses from TString to TRope. Thus it became possible when queries were local and blobs weren't huge that TEvVGetResult delivers rope pointing to Fresh segment of data. And usage of unsafe rope manipulators (introduced as drop-in replacement for const_cast<char*>(TString::data())) led to changing data inside the Fresh segment, instead of making on-demand copy and changing it.

For this bug to happen the following conditions needed to be met for original blob to be possibly corrupted:

  1. Blob must have been written recently (still in fresh segment, not compacted to L0)
  2. Blob must be small
  3. Patch query must have requested blob part from process-local VDisk

Copy link

github-actions bot commented Jan 31, 2024

2024-01-31 11:34:22 UTC Pre-commit check for 6a5af67 has started.
2024-01-31 11:34:25 UTC Build linux-x86_64-relwithdebinfo is running...
🟢 2024-01-31 12:43:18 UTC Build successful.
2024-01-31 12:43:32 UTC Tests are running...
🔴 2024-01-31 14:20:28 UTC Some tests failed, follow the links below.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
60345 51047 0 3 9268 27

Copy link

github-actions bot commented Jan 31, 2024

2024-01-31 11:34:40 UTC Pre-commit check for 6a5af67 has started.
2024-01-31 11:34:42 UTC Build linux-x86_64-release-asan is running...
🟢 2024-01-31 12:45:35 UTC Build successful.
2024-01-31 12:45:48 UTC Tests are running...
🔴 2024-01-31 14:29:13 UTC Some tests failed, follow the links below.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
16025 15930 0 11 55 29

@alexvru alexvru merged commit 18a05f7 into ydb-platform:main Jan 31, 2024
@alexvru alexvru deleted the vdisk/fix-bug-with-patching branch January 31, 2024 12:21
This was referenced Jan 31, 2024
@pavelvelikhov pavelvelikhov mentioned this pull request Feb 3, 2024
@niksaveliev niksaveliev mentioned this pull request Feb 5, 2024
@starlinskiy starlinskiy mentioned this pull request Feb 12, 2024
@vitstn vitstn mentioned this pull request Feb 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants