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.

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

crimson/os/seastore: set ExtentPlacementManager::allocated_to before rolling segments #43492

Merged
merged 1 commit into from Oct 12, 2021
Merged

crimson/os/seastore: set ExtentPlacementManager::allocated_to before rolling segments #43492

merged 1 commit into from Oct 12, 2021

Conversation

xxhdx1985126
Copy link
Contributor

There are circumstances in which a transaction that are supposed to roll the current segment
is invalidated after it finished writing and before it rolls the segment. If we don't set
ExtentPlacementManager::allocated_to in this situation, another transaction can try to write
to the old "allocated_to" position, which would cause an invalid write error

Fixes: https://tracker.ceph.com/issues/52894
Signed-off-by: Xuehan Xu xxhdx1985126@gmail.com

Checklist

  • References tracker ticket
  • Updates documentation if necessary
  • Includes tests for new functionality or reproducer for bug
Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test dashboard cephadm
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox

@xxhdx1985126 xxhdx1985126 requested a review from a team as a code owner October 12, 2021 02:20
cyx1231st
cyx1231st approved these changes Oct 12, 2021
View reviewed changes
Copy link
Member

@cyx1231st cyx1231st left a comment

Choose a reason for hiding this comment

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

LGTM with some nits.

src/crimson/os/seastore/extent_placement_manager.cc Outdated Show resolved Hide resolved
athanatos
athanatos approved these changes Oct 12, 2021
View reviewed changes
Copy link
Contributor

@athanatos athanatos left a comment

Choose a reason for hiding this comment

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

Same comment as @cyx1231st, otherwise looks good.

@xxhdx1985126
crimson/os/seastore: set ExtentPlacementManager::allocated_to before …
460f6ca 
…rolling segments

There are circumstances in which a transaction that are supposed to roll the current segment
is invalidated after it finished writing and before it rolls the segment. If we don't set
ExtentPlacementManager::allocated_to in this situation, another transaction can try to write
to the old "allocated_to" position, which would cause an invalid write error

Fixes: https://tracker.ceph.com/issues/52894
Signed-off-by: Xuehan Xu <xxhdx1985126@gmail.com>
@xxhdx1985126
Copy link
Contributor Author

@athanatos @cyx1231st done

@athanatos athanatos merged commit 79a5ded into ceph:master Oct 12, 2021
@xxhdx1985126 xxhdx1985126 deleted the wip-52894 branch April 23, 2024 02:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@athanatos athanatos athanatos approved these changes

@cyx1231st cyx1231st cyx1231st approved these changes

@liu-chunmei liu-chunmei liu-chunmei approved these changes

Assignees
No one assigned
Labels
crimson
Projects
None yet
Milestone
No milestone
4 participants
@xxhdx1985126 @athanatos @cyx1231st @liu-chunmei