Skip to content

Commit

Permalink
ShardedLmdb takes max size in bytes, not pages (#8038)
Browse files Browse the repository at this point in the history
  • Loading branch information
illicitonion authored Jul 11, 2019
1 parent d539855 commit 503f17a
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 11 deletions.
12 changes: 12 additions & 0 deletions src/rust/engine/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion src/rust/engine/fs/store/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -910,7 +910,7 @@ mod local {
// by python, but they're not, so...
file_dbs: ShardedLmdb::new(
files_root.clone(),
1024 * 1024 * 1024 * 1024 / 10,
1024 * 1024 * 1024 * 1024,
executor.clone(),
)
.map(Arc::new),
Expand Down
1 change: 1 addition & 0 deletions src/rust/engine/sharded_lmdb/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,6 @@ futures = "^0.1.16"
hashing = { path = "../hashing" }
lmdb = { git = "https://github.com/pantsbuild/lmdb-rs.git", rev = "06bdfbfc6348f6804127176e561843f214fc17f8" }
log = "0.4"
page_size = "0.4"
task_executor = { path = "../task_executor" }
tempfile = "3"
23 changes: 13 additions & 10 deletions src/rust/engine/sharded_lmdb/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,25 +49,25 @@ pub struct ShardedLmdb {
// First Database is content, second is leases.
lmdbs: HashMap<u8, (Arc<Environment>, Database, Database)>,
root_path: PathBuf,
max_size: usize,
max_size_bytes: usize,
executor: task_executor::Executor,
}

impl ShardedLmdb {
// max_size is the maximum size the databases together will be allowed to grow to.
// max_size is the maximum size the databases together will be allowed to grow to in bytes.
// When calling this function, we will attempt to allocate that much virtual (not resident) memory
// for the mmap; in theory it should be possible not to bound this, but in practice we see travis
// occasionally fail tests because it's unable to allocate virtual memory if we set this too high,
// and we have too many tests running concurrently or close together.
pub fn new(
root_path: PathBuf,
max_size: usize,
max_size_bytes: usize,
executor: task_executor::Executor,
) -> Result<ShardedLmdb, String> {
trace!("Initializing ShardedLmdb at root {:?}", root_path);
let mut lmdbs = HashMap::new();

for (env, dir, fingerprint_prefix) in ShardedLmdb::envs(&root_path, max_size)? {
for (env, dir, fingerprint_prefix) in ShardedLmdb::envs(&root_path, max_size_bytes)? {
trace!("Making ShardedLmdb content database for {:?}", dir);
let content_database = env
.create_db(Some("content"), DatabaseFlags::empty())
Expand Down Expand Up @@ -97,12 +97,15 @@ impl ShardedLmdb {
Ok(ShardedLmdb {
lmdbs,
root_path,
max_size,
max_size_bytes,
executor,
})
}

fn envs(root_path: &Path, max_size: usize) -> Result<Vec<(Environment, PathBuf, u8)>, String> {
fn envs(
root_path: &Path,
max_size_bytes: usize,
) -> Result<Vec<(Environment, PathBuf, u8)>, String> {
let mut envs = Vec::with_capacity(0x10);
for b in 0x00..0x10 {
let fingerprint_prefix = b << 4;
Expand All @@ -113,15 +116,15 @@ impl ShardedLmdb {
fs::safe_create_dir_all(&dir)
.map_err(|err| format!("Error making directory for store at {:?}: {:?}", dir, err))?;
envs.push((
ShardedLmdb::make_env(&dir, max_size)?,
ShardedLmdb::make_env(&dir, max_size_bytes)?,
dir,
fingerprint_prefix,
));
}
Ok(envs)
}

fn make_env(dir: &Path, max_size: usize) -> Result<Environment, String> {
fn make_env(dir: &Path, max_size_bytes: usize) -> Result<Environment, String> {
Environment::new()
// NO_SYNC
// =======
Expand Down Expand Up @@ -156,7 +159,7 @@ impl ShardedLmdb {
.set_flags(EnvironmentFlags::NO_SYNC | EnvironmentFlags::NO_TLS)
// 2 DBs; one for file contents, one for leases.
.set_max_dbs(2)
.set_map_size(max_size)
.set_map_size(max_size_bytes / page_size::get())
.open(dir)
.map_err(|e| format!("Error making env for store at {:?}: {}", dir, e))
}
Expand Down Expand Up @@ -254,7 +257,7 @@ impl ShardedLmdb {

#[allow(clippy::identity_conversion)] // False positive: https://github.com/rust-lang/rust-clippy/issues/3913
pub fn compact(&self) -> Result<(), String> {
for (env, old_dir, _) in ShardedLmdb::envs(&self.root_path, self.max_size)? {
for (env, old_dir, _) in ShardedLmdb::envs(&self.root_path, self.max_size_bytes)? {
let new_dir = TempDir::new_in(old_dir.parent().unwrap()).expect("TODO");
env
.copy(new_dir.path(), EnvironmentCopyFlags::COMPACT)
Expand Down

0 comments on commit 503f17a

Please sign in to comment.