Skip to content

Commit

Permalink
fix: always grow database when asked to resize (tari-project#3313)
Browse files Browse the repository at this point in the history
Description
---
When a resize is detected as needed, we no longer need to do threshold
checking. Simply grow the database by the configured amount.

Increase the number of MAX_RESIZES in a transaction to accomodate very
large transactions (interestingly, needed during rewind-blockchain for >
1000 blocks, depending on configured resize growth).

- Fix incorrect error log in lmdb helper functions.
- Check for MAP_FULL in lmdb_replace

Motivation and Context
---
Bug fixes

How Has This Been Tested?
---
using rewind-blockchain on base node
  • Loading branch information
sdbondi authored and Cifko committed Sep 10, 2021
1 parent 529195d commit 7e6eae8
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 12 deletions.
2 changes: 2 additions & 0 deletions base_layer/core/src/chain_storage/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,8 @@ pub enum ChainStorageError {
KeyExists { table_name: &'static str, key: String },
#[error("Database resize required")]
DbResizeRequired,
#[error("DB transaction was too large ({0} operations)")]
DbTransactionTooLarge(usize),
}

impl ChainStorageError {
Expand Down
13 changes: 9 additions & 4 deletions base_layer/core/src/chain_storage/lmdb_db/lmdb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,15 +115,15 @@ where
{
let val_buf = serialize(val)?;
txn.access().put(&db, key, &val_buf, put::Flags::empty()).map_err(|e| {
error!(
target: LOG_TARGET,
"Could not insert value into lmdb transaction: {:?}", e
);
if let lmdb_zero::Error::Code(code) = &e {
if *code == lmdb_zero::error::MAP_FULL {
return ChainStorageError::DbResizeRequired;
}
}
error!(
target: LOG_TARGET,
"Could not insert value into lmdb transaction: {:?}", e
);
ChainStorageError::AccessError(e.to_string())
})
}
Expand All @@ -136,6 +136,11 @@ where
{
let val_buf = serialize(val)?;
txn.access().put(&db, key, &val_buf, put::Flags::empty()).map_err(|e| {
if let lmdb_zero::Error::Code(code) = &e {
if *code == lmdb_zero::error::MAP_FULL {
return ChainStorageError::DbResizeRequired;
}
}
error!(
target: LOG_TARGET,
"Could not replace value in lmdb transaction: {:?}", e
Expand Down
12 changes: 8 additions & 4 deletions base_layer/core/src/chain_storage/lmdb_db/lmdb_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1335,7 +1335,7 @@ impl BlockchainBackend for LMDBDatabase {

let mark = Instant::now();
// Resize this many times before assuming something is not right
const MAX_RESIZES: usize = 3;
const MAX_RESIZES: usize = 5;
for i in 0..MAX_RESIZES {
let num_operations = txn.operations().len();
match self.apply_db_transaction(&txn) {
Expand All @@ -1350,13 +1350,17 @@ impl BlockchainBackend for LMDBDatabase {
return Ok(());
},
Err(ChainStorageError::DbResizeRequired) => {
info!(target: LOG_TARGET, "Database resize required (iteration: {})", i);
info!(
target: LOG_TARGET,
"Database resize required (resized {} time(s) in this transaction)",
i + 1
);
// SAFETY: This depends on the thread safety of the caller. Technically, `write` is unsafe too
// however we happen to know that `LmdbDatabase` is wrapped in an exclusive write lock in
// BlockchainDatabase, so we know there are no other threads taking out LMDB transactions when this
// is called.
unsafe {
LMDBStore::resize_if_required(&self.env, &self.env_config)?;
LMDBStore::resize(&self.env, &self.env_config)?;
}
},
Err(e) => {
Expand All @@ -1366,7 +1370,7 @@ impl BlockchainBackend for LMDBDatabase {
}
}

Err(ChainStorageError::DbResizeRequired)
Err(ChainStorageError::DbTransactionTooLarge(txn.operations().len()))
}

fn fetch(&self, key: &DbKey) -> Result<Option<DbValue>, ChainStorageError> {
Expand Down
32 changes: 28 additions & 4 deletions infrastructure/storage/src/lmdb_store/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ impl LMDBStore {
);

if size_left_bytes <= config.resize_threshold_bytes {
env.set_mapsize(env_info.mapsize + config.grow_size_bytes)?;
Self::resize(env, config)?;
debug!(
target: LOG_TARGET,
"({}) LMDB size used {:?} MB, environment space left {:?} MB, increased by {:?} MB",
Expand All @@ -424,6 +424,31 @@ impl LMDBStore {
}
Ok(())
}

/// Grows the LMDB environment by the configured amount
///
/// # Safety
/// This may only be called if no write transactions are active in the current process. Note that the library does
/// not check for this condition, the caller must ensure it explicitly.
///
/// http://www.lmdb.tech/doc/group__mdb.html#gaa2506ec8dab3d969b0e609cd82e619e5
pub unsafe fn resize(env: &Environment, config: &LMDBConfig) -> Result<(), LMDBError> {
let env_info = env.info()?;
let current_mapsize = env_info.mapsize;
env.set_mapsize(current_mapsize + config.grow_size_bytes)?;
let env_info = env.info()?;
let new_mapsize = env_info.mapsize;
debug!(
target: LOG_TARGET,
"({}) LMDB MB, mapsize was grown from {:?} MB to {:?} MB, increased by {:?} MB",
env.path()?.to_str()?,
current_mapsize / BYTES_PER_MB,
new_mapsize / BYTES_PER_MB,
config.grow_size_bytes / BYTES_PER_MB,
);

Ok(())
}
}

#[derive(Clone)]
Expand All @@ -442,8 +467,7 @@ impl LMDBDatabase {
K: AsLmdbBytes + ?Sized,
V: Serialize,
{
const MAX_RESIZES: usize = 3;

const MAX_RESIZES: usize = 5;
let value = LMDBWriteTransaction::convert_value(value)?;
for _ in 0..MAX_RESIZES {
match self.write(key, &value) {
Expand All @@ -456,7 +480,7 @@ impl LMDBDatabase {
// SAFETY: We know that there are no open transactions at this point because ...
// TODO: we don't guarantee this here but it works because the caller does this.
unsafe {
LMDBStore::resize_if_required(&self.env, &self.env_config)?;
LMDBStore::resize(&self.env, &self.env_config)?;
}
},
Err(e) => return Err(e.into()),
Expand Down

0 comments on commit 7e6eae8

Please sign in to comment.