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

Lock OMAP file before changing it to avoid corruption in multi-gateway environment #252

Closed
wants to merge 0 commits into from
Closed

Lock OMAP file before changing it to avoid corruption in multi-gateway environment #252

wants to merge 0 commits into from

Conversation

gbregman
Copy link
Contributor

@gbregman gbregman commented Oct 4, 2023 •
edited
Loading

Lock OMAP file before changing it to avoid corruption in multi-gateway environment

Fixes #56

@gbregman gbregman requested review from baum, sskaur, oritwas and caroav October 4, 2023 14:16
@gbregman
Copy link
Contributor Author

gbregman commented Oct 4, 2023

I had to open this PR after the original PR got closed by github when I merged the last commit into my repository. The original PR was #243

@gbregman gbregman changed the title Lock OMAP file before changing it to avoid corruption in multi-gateway environemnt Lock OMAP file before changing it to avoid corruption in multi-gateway environment Oct 4, 2023
baum
baum reviewed Oct 9, 2023
View reviewed changes
ceph-nvmeof.conf Outdated Show resolved Hide resolved
baum
baum reviewed Oct 9, 2023
View reviewed changes
control/grpc.py Outdated Show resolved Hide resolved
baum
baum reviewed Oct 9, 2023
View reviewed changes
control/state.py Outdated Show resolved Hide resolved
baum
baum reviewed Oct 9, 2023
View reviewed changes
control/grpc.py Outdated Show resolved Hide resolved
@oritwas
Copy link
Member

oritwas commented Oct 9, 2023

@gbregman please add in the commit and PR message more details on the issue you are resolving
Why do we need an OMAP lock?
Why not use atomic compare and set command?
I also noticed that the gateway is multithreaded and we need to handle that (here maybe we will need locking)?

@gbregman
Copy link
Contributor Author

gbregman commented Oct 9, 2023

@oritwas this PR is based on the solution suggested by Jonas for the multi gateway issues with OMAP files. As we deal with a multi process environment we need a mechanism which is visible to all processes, this is why we use the lock mechanism of Rados. I deal with the multi-threading issue in the gateway. We can get to the OMAP updating function from two places and I distinguish between the two.

@gbregman
Copy link
Contributor Author

gbregman commented Oct 9, 2023

Concurrent Requests.pdf

@gbregman gbregman closed this Oct 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@baum baum baum left review comments

@sskaur sskaur Awaiting requested review from sskaur

@oritwas oritwas Awaiting requested review from oritwas

@caroav caroav Awaiting requested review from caroav

Assignees
No one assigned
Labels
None yet
Projects
NVMe-oF
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Failure on OMAP add/remove -> update & retry
3 participants
@gbregman @oritwas @baum