Skip to content

Commit

Permalink
Reintroduce RoTxn::commit: fix database opening read-only database.
Browse files Browse the repository at this point in the history
Opening of databases with Env::open_database() without previous Env in memory
with a RoTxn lead to the following error:
`Io(Os { code: 22, kind: InvalidInput, message: "Invalid argument" })`

It's due to the fact that RoTxn are not commited so the env->me_numdbs cannot be
updated and will be set to CORE_DB so 2.

It can trigger from multi-process setup, read-only opening, closing write
access, and re-opening from the same process.

Thanks to @Kerollmops for the typo and review.

Co-authored-by: Cl茅ment Renault <renault.cle@gmail.com>
  • Loading branch information
2 people authored and darnuria committed Jul 11, 2023
1 parent 9630a90 commit be032ca
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 0 deletions.
59 changes: 59 additions & 0 deletions heed/src/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,15 @@ impl Env {
/// LMDB have an important restriction on the unnamed database when named ones are opened,
/// the names of the named databases are stored as keys in the unnamed one and are immutable,
/// these keys can only be read and not written.
///
/// ## Lmdb read only access of existing database
///
/// In the case of accessing a a database in a read-only manner from an other process that the one
/// where you wrote you might need to call manually `RoTxn::commit`,
/// to get metadata and databases handles opened shared with the global [Env] handle.
///
/// If not done you might raise `Io(Os { code: 22, kind: InvalidInput, message: "Invalid argument" })`
/// known as `EINVAL` from LMDB on access functions.
pub fn open_database<KC, DC>(
&self,
rtxn: &RoTxn,
Expand Down Expand Up @@ -956,4 +965,54 @@ mod tests {

assert_eq!(10 * 4096, env.info().map_size);
}

#[test]
fn open_read_only_without_no_env_opened_before() {
let expected_data0 = "Data Expected db0";
let dir = tempfile::tempdir().unwrap();

{
// We really need this env to be dropped before the read-only access.
let env = EnvOpenOptions::new()
.map_size(16 * 1024 * 1024 * 1024) // 10MB
.max_dbs(32)
.open(&dir.path())
.unwrap();
let mut wtxn = env.write_txn().unwrap();
let database0 = env.create_database::<Str, Str>(&mut wtxn, Some("shared0")).unwrap();

wtxn.commit().unwrap();
let mut wtxn = env.write_txn().unwrap();
database0.put(&mut wtxn, "shared0", expected_data0).unwrap();
wtxn.commit().unwrap();
// We also really need that no other env reside in memory in other thread doing tests.
env.prepare_for_closing().wait();
}

{
// Open now we do a read-only opening
let env = EnvOpenOptions::new()
.map_size(16 * 1024 * 1024 * 1024) // 10MB
.max_dbs(32)
.open(&dir.path())
.unwrap();
let database0 = {
let rtxn = env.read_txn().unwrap();
let database0 = env.open_database::<Str, Str>(&rtxn, Some("shared0")).unwrap().unwrap();
// This commit is mandatory if not committed you might get
// Io(Os { code: 22, kind: InvalidInput, message: "Invalid argument" })
rtxn.commit().unwrap();
database0
};

{
// If we didn't commited the opening it might fail with EINVAL.
let rtxn = env.read_txn().unwrap();
let value = database0.get(&rtxn, "shared0").unwrap().unwrap();
assert_eq!(value, expected_data0);
}

env.prepare_for_closing().wait();
}
}
}
16 changes: 16 additions & 0 deletions heed/src/txn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,22 @@ impl<'e> RoTxn<'e> {
pub(crate) fn env_mut_ptr(&self) -> *mut ffi::MDB_env {
self.env.env_mut_ptr()
}

/// Commit a read transaction.
///
/// Synchronizing some [Env] metadata with the global handle.
///
/// ## Lmdb
///
/// It's mandatory in a multi-process setup to call [RoTxn::commit] upon readonly database opening.
/// After the transaction opening the a database is `drop`ed. the next transaction might return
/// `Io(Os { code: 22, kind: InvalidInput, message: "Invalid argument" })`
/// known as `EINVAL` on access functions.
pub fn commit(mut self) -> Result<()> {
let result = unsafe { mdb_result(ffi::mdb_txn_commit(self.txn)) };
self.txn = ptr::null_mut();
result.map_err(Into::into)
}
}

impl Drop for RoTxn<'_> {
Expand Down

0 comments on commit be032ca

Please sign in to comment.