Skip to content

Introduce the Env::nested_read_txn from an RwTxn #307

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

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

Kerollmops
Copy link
Member

@Kerollmops Kerollmops commented Jan 23, 2025

This PR is a first attempt at patching LMDB to generate multiple RoTxn from an RwTxn and, therefore, be able to read the content of an uncommitted RwTxn in parallel. We can run many algorithms in Meilisearch to process the written content and generate new data structures before committing the transaction.

// opening a write transaction
let mut wtxn = env.write_txn()?;

// [use the RwTxn to write into the env]

// opening multiple read-only transactions without committing beforehand
let rtxns = (0..1000).map(|_| env.nested_read_txn(&wtxn)).collect::<heed::Result<Vec<_>>>()?;

// [use the RoTxns and move them onto different threads]

This first attempt uses a broken fork of LMDB that ignores the number of nested write transactions associated with the parent one. Therefore, it lets heed create multiple nested write transactions from the parent one (dangerous). We store those write transactions in RoTxns shells to disallow writing anything with them at the compilation level.

However, while thinking about this, it is probably still possible to open databases (already existing) and commit them from multiple threads concurrently. It should be either disallowed or will, anyway, be fixed by the correct LMDB patch later.

To fix the Windows CI we will have to use the experimental stdatomic headers. Actually, no. We had to enable the /std:c11 and /experimental:c11atomics features.

Note that the CI is broken, but it works locally, so it's maybe a submodule issue (it's always a submodule issue) 🤔 It was...

To do

  • Speed up the new nested-rtxns example in the CI (build it in release)

@Kerollmops Kerollmops added the experimental Is unstable and must be tested more thoroughly label Mar 6, 2025
@Kerollmops Kerollmops force-pushed the allow-nested-rtxn-from-wtxn branch from 9a94c8a to 1d1926b Compare March 7, 2025 13:13
@Kerollmops Kerollmops force-pushed the allow-nested-rtxn-from-wtxn branch 2 times, most recently from 1a8a21c to b7ebf74 Compare April 1, 2025 13:16
@Kerollmops Kerollmops force-pushed the allow-nested-rtxn-from-wtxn branch from ab18780 to 0c6793c Compare April 1, 2025 13:18
@Kerollmops Kerollmops force-pushed the allow-nested-rtxn-from-wtxn branch from bdf9c69 to 81ae19f Compare April 1, 2025 14:12
@Kerollmops Kerollmops force-pushed the allow-nested-rtxn-from-wtxn branch from 5fcc354 to a54defd Compare April 1, 2025 15:11
@Kerollmops Kerollmops force-pushed the allow-nested-rtxn-from-wtxn branch from a54defd to 05d9f22 Compare April 1, 2025 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
experimental Is unstable and must be tested more thoroughly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant