Read-only database opening does not work on a read-only context

[darnuria]

This pull-request purpose a non-regrettion+functional test for opening read_only databases and fix.

The issue was already fixed in #10 but regressed in c40afad6bbd223832a2f86c38702313beb78366bd30bbfcd6d014e8cc651dafe L30-L39

EDIT: edited PR to only describe the fix+test made a separate issue #183

[darnuria]

(rebased against main)

[darnuria]

Hello, if you want to discuss it I am on element (preferred) or discord. :)

[Kerollmops]

Hey @darnuria 👋

I will not have much time to help on this, but if you want more direct messages you can create a thread in #discussion of the Meilisearch Discord channel 😊

[darnuria]

Hello, no problem! But it's a major blocker for release of 0.20.x since the open_database API is disfunctionnal (at least in multiprocess env). Will join the discord today! :)

EDIT: Not in a rush take time ! :D

[Kerollmops]

Hey @darnuria 👋

Thank you for this PR. However, could you make the tests only run on the UNIX machines and add documentation on the RoTxn::commit method to make its utility explicit (and check English quality, before submitting 😅)?

[darnuria]

Thank you for this PR. However, could you make the tests only run on the UNIX machines

About the test (on my computer) it's quite complicated got multiprocess-deadlock since cargo test does thread multi-threaded within the same environment, upon forking we also... fork inherit the memory space of the parent and share the lock overs the environments. (It's not a direct problem if you don't open with heed not from the parent process-tree).

So I should force one-thread testing, fun that it passed CI!

the straceing lead to that dead_lock between close_env test and the new one!

[pid 266355] futex(0x5598f91a2b58, FUTEX_WAIT_BITSET_PRIVATE, 4294967295, {tv_sec=54071, tv_nsec=410305923}, FUTEX_BITSET_MATCH_ANY) = -1 ETIMEDOUT (Connection timed out)

   Compiling heed v0.20.0-alpha.2 ./heed/heed)
    Finished test [unoptimized + debuginfo] target(s) in 1.31s
     Running unittests src/lib.rs (target/debug/deps/heed-56be63de59924409)

running 17 tests
test env::tests::create_database_without_commit ... ok
test env::tests::open_database_with_nosubdir ... ok
test env::tests::open_env_with_named_path ... ok
test env::tests::reopen_env_with_different_options_is_err ... ok
test iter::tests::prefix_iter_last ... ok
test iter::tests::iter_last ... ok
test env::tests::open_already_existing_database ... ok
test env::tests::open_database_with_writemap_flag ... ok
test env::tests::resize_database ... ok
test mdb::lmdb_error::test::test_description ... ok
test tests::error_is_send_sync ... ok
test iter::tests::prefix_iter_with_byte_255 ... ok
test iter::tests::range_iter_last ... ok
test iter::tests::rev_prefix_iter_last ... ok
test iter::tests::rev_range_iter_last ... ok
test env::tests::close_env ... ok
test env::tests::open_already_existing_database_multiprocess has been running for over 60 seconds

Maybe just documentation about the txn::commit()? ^_^'

Will exclude that it's due to the tempfile that keep something open just to be sure.

The more 'ideal' solution would be to add examples to CI like suggest in #146 and check return code to have "read-world-complex test" but it also-show an issue with the current design in the case of a process-fork and share locks.

[Kerollmops]

Is it related to this part of the documentation, and can we do anything about it apart from documenting it?

[darnuria]

Should be ok, force-pushed some typo and clippy.

[darnuria]

Yeah found a way to make the test relying on the same process space! It needs to be the only one having an Env I hope it didn't cause too much problems. But it's more usable than the forking way. :/

Edit: Removed the paste, I found a way to test what happens if open_database is not commited

[Kerollmops]

Thank you for PR. I noticed some little English mistakes and a test to change a little bit 👌

[Kerollmops]

Thank you infinitely for your help on that and the documentation addition 😍