Add the POSIX semaphore feature from LMDB #140
Conversation
|
Also not sure whether this PR should be made to main or v0.12; if I should switch it to v0.12 (to be used with milli) please let me know. |
There was a problem hiding this comment.
You could investigate using POSIX semaphores within milli/meilisearch as well for the possible speed improvements they bring, but that is outside the scope of this PR.
This comment from Howard can be very interesting! Using POSIX semaphores can bring new issues. There are information about the C defines that we can use with LMDB in the Makefile (more in the mdb.c).
Also not sure whether this PR should be made to main or v0.12; if I should switch it to v0.12 (to be used with milli) please let me know.
Yes, you must start from the branch meilisearch and milli uses which is v0.12.
| # Whether to tell LMDB to use POSIX semaphores during compilation | ||
| # (instead of System V semaphores). | ||
| # POSIX semaphores are required for Apple's App Sandbox on iOS & macOS, | ||
| # and are possibly faster and more appropriate for single-process use. | ||
| posix-sem = ["lmdb-rkv-sys/posix-sem"] |
There was a problem hiding this comment.
Would it be possible to change the documentation a little bit here? Could you add a link to this section in the doc, please?
There was a problem hiding this comment.
Looks like I am going to have to recreate the PR because changing the base didn't work as I wanted. Will update the docs in the new PR.
Very true. I will note that if building meili into a Docker image, however, I would imagine that it would make sense to use POSIX semaphores even with that drawback (specifically, not cleaning up on a crash) since the container would just be restarted and the semaphore(s) would consequently be closed/reset as soon as the container exits and restarts. However, if building meili into just an executable, and not in a container, it would still make sense to use SysV semaphores (at least not on iOS/macOS). All just depends on the target platform/use case.
Noted; will switch this PR over to |
Before looking at this PR, please see meilisearch/lmdb-rs#13. It is required before this PR can be merged.
This PR adds LMDB's POSIX semaphore feature to heed. This change allows iOS and macOS builds to be compliant with Apple's App Sandbox (necessary for distribution in the App Store), in addition to possible speed improvements brought upon by the POSIX semaphores.
You could investigate using POSIX semaphores within milli/meilisearch as well for the possible speed improvements they bring, but that is outside the scope of this PR.