-
Notifications
You must be signed in to change notification settings - Fork 454
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
Obtain a lock on filePathPrefix
on dbnode startup. Close #641
#1376
Obtain a lock on filePathPrefix
on dbnode startup. Close #641
#1376
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks really good, just a few minor comments
Also let me know if I need to squash. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, could you fix the merge conflict? We've almost figured out how to run C.I for P.Rs from external contributors but we're not quite there yet but I can run C.I and make sure it passes
e2a8a64
to
9c4c276
Compare
9c4c276
to
7ba2bcc
Compare
Nice, let me know if you need a test subject :) The conflict is now resolved, commits squashed. |
Codecov Report
@@ Coverage Diff @@
## master #1376 +/- ##
=========================================
- Coverage 70.7% 49.7% -21%
=========================================
Files 827 669 -158
Lines 71224 59496 -11728
=========================================
- Hits 50384 29623 -20761
- Misses 17539 27334 +9795
+ Partials 3301 2539 -762
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Green build: https://buildkite.com/uberopensource/m3-monorepo-ci/builds/196
Congrats and thank you :) If you're interested in continuing to work on M3 shoot me a message or an email and we can pick a more challenging next task!
As discussed in #641, with this PR a lock on
filePathPrefix
will be acquired when starting a db node. Two dbnode processes will not be able to run with the samefilePathPrefix
.In case another process already has a lock, this error message would be printed:
When the dbnode exits gracefully, the lock file will be removed (using a
defer
).When the dbnode doesn't exit gracefully, another dbnode process will be able to obtain the lock. A bit more details: there's a distinction b/w the lock file which resides on the FS, and the lock which resides in memory; when the dbnode is killed ungracefully the lock file will remain on the FS, but the lock will be removed from memory; the next time a dbnode starts, it will be able to acquire a lock despite the fact that the lock file exists.
@richardartoul please let me know if this PR can be improved.