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.

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

Node & Session aware locking #62

Open
sam-ulrich1 opened this issue Mar 14, 2022 · 9 comments
Open

Node & Session aware locking #62

sam-ulrich1 opened this issue Mar 14, 2022 · 9 comments

Comments

@sam-ulrich1
Copy link
Contributor

I am looking at implementing a node and session aware version of this lib. Would you be interested in a PR instead?

Goal:
In the case that a node is improperly terminated, deadlocks on the fs can be detected and automatically repaired at runtime.

Changes:

  • Lock contains an id that is unique to each node but persists across many runs (sessions)
  • Lock contains an id that is unique to the current session (maybe nanos of start)
  • Lock will auto detect dead locks that exist from a prior session of the same node and remove the deadlock
@sam-ulrich1
Copy link
Contributor Author

@chrisgilmerproj I completed my modifications. Are you interested in a PR?

@chrisgilmerproj
Copy link
Contributor

Yes please!

@sam-ulrich1
Copy link
Contributor Author

Great to hear! I'll get it over by the end of the day

@sam-ulrich1
Copy link
Contributor Author

So I private forked it and made the mods here https://github.com/Gage-Technologies/safelock

I'm going to personally fork it and copy the changes so I can PR easily

@sam-ulrich1
Copy link
Contributor Author

@chrisgilmerproj not sure if you saw it but here's the pr #63
No worries if you're just busy!

@sam-ulrich1
Copy link
Contributor Author

sam-ulrich1 commented Mar 29, 2022

@chrisgilmerproj I found a bug in the latest merge. The usage of newline for separation can have collisions with the little endian endcoded session ids. It's common enough that after about 4 million file locks I've found 50k deadlocks. I've traded out the separator for __::__ in our repo. I can send another PR soon if you'd like

@chrisgilmerproj
Copy link
Contributor

No problem @sam-ulrich1 . Send a PR and I'll approve it.

@sam-ulrich1
Copy link
Contributor Author

@chrisgilmerproj #72

@chrisgilmerproj
Copy link
Contributor

Reviewed and merged!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants