Skip to content

Commit

Permalink
fix(buffers): Work around a LevelDB mechanism mismatch (#7398)
Browse files Browse the repository at this point in the history
* Work around a LevelDB mechanism mismatch

This commit moves the computation of the byte size of a disk buffer into a
separate function with a separate, temporary LevelDB handle. When we iterate
LevelDB it maps up to 1000 LDB files into vector's address space, which is fine
for a database but imposes a large memory burden on vector users. See
google/leveldb#866 for details. Unfortunately LevelDB
is not configurable in this regard without forking their code base and so we are
temporarily working around the issue, kind of.

The pattern will be this. At startup vector will scan each of its LDB files for
each disk buffer (see #7380) and map up to 1000 LDB files into vector's address
space, consuming however much memory to do this. Once `db_initial_size` returns
the LevelDB handle will drop and each file will be unmapped, reducing vector's
overall memory use. This will NOT resolve OOM issues for our memory constrained
users but it WILL reduce vector's post-startup memory burden, at least until
LevelDB maps in more LDB files which will happen gradually.

REF #7246

Signed-off-by: Brian L. Troutwine <brian@troutwine.us>

* clippy ding

Signed-off-by: Brian L. Troutwine <brian@troutwine.us>
  • Loading branch information
blt committed May 11, 2021
1 parent b8a0cc0 commit 848d922
Showing 1 changed file with 28 additions and 2 deletions.
30 changes: 28 additions & 2 deletions src/buffers/disk/leveldb_buffer.rs
Expand Up @@ -14,7 +14,7 @@ use std::{
collections::VecDeque,
convert::TryInto,
mem::size_of,
path::PathBuf,
path::{Path, PathBuf},
pin::Pin,
sync::{
atomic::{AtomicUsize, Ordering},
Expand Down Expand Up @@ -327,6 +327,31 @@ impl Reader {

pub struct Buffer;

/// Read the byte size of the database
///
/// There is a mismatch between LevelDB's mechanism and vector's. While
/// vector would prefer to keep as little in-memory as possible LevelDB,
/// being a database, has the opposite consideration. As such it may mmap
/// 1000 of its LDB files into vector's address space at a time with no
/// ability for us to change this number. See
/// https://github.com/google/leveldb/issues/866. Because we do need to know
/// the byte size of our store we are forced to iterate through all the LDB
/// files on disk, meaning we impose a huge memory burden on our end users
/// right at the jump in conditions where the disk buffer has filled
/// up. This'll OOM vector, meaning we're trapped in a catch 22.
///
/// This function does not solve the problem -- LevelDB will still map 1000
/// files if it wants -- but we at least avoid forcing this to happen at the
/// start of vector.
fn db_initial_size(path: &Path) -> Result<usize, Error> {
let mut options = Options::new();
options.create_if_missing = true;
let db: Database<Key> = Database::open(&path, options).with_context(|| DataDirOpenError {
data_dir: path.parent().expect("always a parent"),
})?;
Ok(db.value_iter(ReadOptions::new()).map(|v| v.len()).sum())
}

impl super::DiskBuffer for Buffer {
type Writer = Writer;
type Reader = Reader;
Expand All @@ -337,6 +362,8 @@ impl super::DiskBuffer for Buffer {
let max_uncompacted_size = (max_size as f64 * MAX_UNCOMPACTED) as usize;
let max_size = max_size - max_uncompacted_size;

let initial_size = db_initial_size(&path)?;

let mut options = Options::new();
options.create_if_missing = true;

Expand All @@ -355,7 +382,6 @@ impl super::DiskBuffer for Buffer {
tail = if iter.valid() { iter.key().0 + 1 } else { 0 };
}

let initial_size = db.value_iter(ReadOptions::new()).map(|v| v.len()).sum();
let current_size = Arc::new(AtomicUsize::new(initial_size));

let write_notifier = Arc::new(AtomicWaker::new());
Expand Down

0 comments on commit 848d922

Please sign in to comment.