meilisearch
Implement Send on Cursors so we can move them between threads
This PR implements Send on the LMDB cursors struct wrapper. The goal is to move database iterators between threads.
If this PR is merged, we must release a v0.22.1 patch. This change is not breaking.
Hey @hyc 👋
Is it valid to move cursors between threads? Nothing in the documentation seems to prevent that, and nothing in the code prevents it either. Note that cursors are always deallocated when dropped (going out of scope). So, I only fear that we create a cursor on one thread and deallocate it on another. However, allocations are done with malloc, so it looks safe.
Thank you and have a lovely week 🍀
Hey @xav-db 👋
I cannot merge this as-is, as the behavior could break the wrapper and some invariants. However, I suggest you test that independently and tell me how your program is doing. Ultimately, I'd like a review from someone more knowledgeable than me on C stuff and thread safety.
Take care 🌵
Hey @antonilol 👋
I think implementing
Sendshould be fine, the docs start with saying "The library is fully thread-aware".
Unfortunately, you cannot trust the sole documentation of a C library. I don't know what "The library is fully thread-aware" exactly means but here is an example of non-thread awareness. Another is that it is not valid to implement Sync on a Cursor (internal) and use this cursor on multiple threads without synchronization.
Also, if desired,
Synccan be implemented on any type without methods that take&self, for free. See std::sync::Exclusive for more info.
Very interesting, thanks. However, this is still nightly. We cannot rely on this in heed. At least, for now.
Another is that it is not valid to implement
Syncon aCursor(internal) and use this cursor on multiple threads without synchronization.
Isn't this impossible already because to use a cursor, &mut self is needed, which can only exist at one place at a time. This is also the reason why Sync can be implemented on any type that does not take self by reference (&self). ("Every method that takes self by reference does proper synchronization" is vacuously true then)
Very interesting, thanks. However, this is still nightly. We cannot rely on this in heed. At least, for now.
I meant this as an example for how Sync can be implemented for free, Sync works like this since the first release of stable Rust, only this wrapper is unstable.
Unfortunately, you cannot trust the sole documentation of a C library. I don't know what "The library is fully thread-aware" exactly means but here is an example of non-thread awareness.
This is true, we will have to wait until the author of LMDB responds or someone checks the C code cursors use.