search

DataDog / glommio

Glommio is a thread-per-core crate that makes writing highly parallel asynchronous applications in a thread-per-core architecture easier for rustaceans.
Other
2.93k stars 161 forks source link

Implement very simple advisory lock mechanism #664

Open vlovich opened 2 months ago

vlovich commented 2 months ago

What does this PR do?

Introduces very basic advisory lock integration. As I mentioned in the issue, going for a very simple non-fancy approach that only lets you acquire a single advisory lock per file entry.

Motivation

Implementing a database & would like to use a safe version of advisory locks to implement mutual exclusion for writers. It's not possible to implement advisory locking in pure safe code outside because the resulting usage becomes unsound very easily (the Rust advisory-lock crate pretends it's safe but I'd argue that crate is unsound because it doesn't guarantee the usage of the lock is correct within the context of the current process).

Related issues

663

Additional Notes

448 prevents us from implementing "blocking" versions that wait forever for the lock to become available. The reason is that it would require spawning a new thread which would leak memory (it would also be horribly slow & interact poorly with cancellation).

Checklist

[X] I have added unit tests to the code I am submitting [X] My unit tests cover both failure and success scenarios [] If applicable, I have discussed my architecture

glommer commented 1 month ago

Is this ready to go ? Looks good on a new review.

In related news, I am considering removing the clippy tests from CI. I still love the idea, but we struggled a lot already trying to figure out a good way to pin versions to make sure it stays stable, yet it keeps breaking. Do you have any thoughts ?

vlovich commented 1 month ago

I thought it was but need to fix failing tests.

I actually don't mind the clippy stuff in CI. Keeps the codebase consistent which outweighs the nitpickiness annoyance of it for me personally. Probably could do with a pre push hook that runs the required clippy lints and doctests before so that I don't churn so much on it though :)

If we're running clippy on a floating version (eg nightly or latest stable) then we could just not do that and stick to running clippy only on pinned versions at the risk of having more work when bumping the minimum / maximum rust version we build against (+ having downstream projects get more complaints if they're running newer than the version we test against). All options come with trade offs 😃

glommer commented 1 month ago

yeah, the pre-hooks may work. I'll take a look at it later this week. It was supposed to be a pinned version, but somehow it keeps being updated.

vlovich commented 1 month ago

BTW the failure here was just a normal compile error :).