[v23.1.x] cloud_storage: Prevent segment reuploads from adjacent segment merger #9845
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
(cherry picked from commit 3c7cbb9)
It is possible that the reuploaded segment is smaller than predicted. In this case it's not safe to reupload it because adjacent segment merging may try to reupload it second time. This commit blocks reupload if the size doesn't match the result of the manifest scan. (cherry picked from commit dc855e2)
Add new optional parameter to the c-tor. The parameter accepts inclusive end offset of the offset range. The segment_collector is forced to reach the target end offset if it's set. This allows adjacent_segment_merger to reupload offset ranges more precisely since it's no longer allowed to have some slack. (cherry picked from commit 441fb14)
The segment_collector calculates the size of the segment sequence during the initial scan and then it uses it to produce an upload candidate. The 'make_upload_candidate' method is a future with several scheduling points. If the size of the last segment changes during the call the method will return incorrect content length. This commit fixes this by caching the size of the last segment. The problem only affect adjacent segment reuploads. It shouldn't affect compacted segment reupload because the last compacted segment is always rolled and its size can't change. (cherry picked from commit ccb0e0a)
andrwng
approved these changes
Apr 5, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport #9657
This PR fixes #9651
Currently, the
segment_collectoris initialised using desired size of the segment reupload. The adjacent segment merger scans the manifest and finds the right upload candidate (a series of small segments). Then it calculates its size and uses it to createsegment_collector. This is not correct because if the last segment is not aligned perfectly the collector will return a result without one last segment. This last segment will overflow the size target and be discarded.With some configurations (close min and target values for adjacent segment merger) this can lead to a situation when the segment gets re uploaded multiple times.
To avoid this several things were added:
adjacent_segment_mergerchecks the size of the segment run that it gets out of thesegment_collectorbefore uploading. If the size is smaller it discards the result.segment_collectoris updated to accept extra parameter which is used as a target offset for the end of the offset range. If this parameter is set thesegment _collectorwill continue scanning until it will reach the segment that contains the target offset. Then it will adjust the upload so this last segment won't be fully uploaded. Instead, it will be uploaded up to the target offset.Backports Required
Release Notes
Bug Fixes