PR to fix issue #109 (running out of files on OSX) (#112)

* Move to new version of DB and re-enable tests * Create store explicitly, and add file limit when testing * Raise the shell fdlimit on OSX before running tests (#114) Co-authored-by: George Danezis <[email protected]> Co-authored-by: François Garillot <[email protected]>
MystenLabs · Jan 4, 2022 · e29c107 · e29c107 · github-actions · Jan 4, 2022
1 parent d15cdd2
commit e29c107
Show file tree

Hide file tree

Showing 7 changed files with 61 additions and 19 deletions.
diff --git a/fastpay/src/bench.rs b/fastpay/src/bench.rs
@@ -16,6 +16,7 @@ use tokio::{runtime::Builder, time};
 
 use std::env;
 use std::fs;
+use std::sync::Arc;
 use std::thread;
 use strum_macros::EnumString;
 
@@ -121,7 +122,9 @@ impl ClientServerBenchmark {
         let path = dir.join(format!("DB_{:?}", ObjectID::random()));
         fs::create_dir(&path).unwrap();
 
-        let state = AuthorityState::new(committee.clone(), public_auth0, secret_auth0.copy(), path);
+        let store = Arc::new(AuthorityStore::open(path, None));
+        let state =
+            AuthorityState::new(committee.clone(), public_auth0, secret_auth0.copy(), store);
 
         // Seed user accounts.
         let mut rt = Runtime::new().unwrap();

diff --git a/fastpay/src/server.rs b/fastpay/src/server.rs
@@ -10,6 +10,7 @@ use fastx_types::{base_types::*, committee::Committee, object::Object};
 use futures::future::join_all;
 use log::*;
 use std::path::Path;
+use std::sync::Arc;
 use structopt::StructOpt;
 use tokio::runtime::Runtime;
 
@@ -30,11 +31,15 @@ fn make_server(
 
     let committee = Committee::new(committee_config.voting_rights());
 
+    let store = Arc::new(AuthorityStore::open(
+        Path::new(&server_config.authority.database_path),
+        None,
+    ));
     let state = AuthorityState::new(
         committee,
         server_config.authority.address,
         server_config.key.copy(),
-        Path::new(&server_config.authority.database_path),
+        store,
     );
 
     // Load initial states

diff --git a/fastpay_core/Cargo.toml b/fastpay_core/Cargo.toml
@@ -6,6 +6,7 @@ publish = false
 edition = "2018"
 
 [dependencies]
+rocksdb = "0.17.0"
 anyhow = "1.0"
 bcs = "0.1.2"
 failure = "0.1.8"
@@ -23,4 +24,7 @@ move-core-types = { git = "https://github.com/diem/diem", rev="346301f33b3489bb4
 move-vm-runtime = { git = "https://github.com/diem/diem", rev="346301f33b3489bb4e486ae6c0aa5e030223b492" }
 
 
-typed-store = { git = "https://github.com/MystenLabs/mysten-infra", rev = "b6bb469e0752222b1c990b1d8f39c69d4dbf12a9" }
+typed-store = { git = "https://github.com/MystenLabs/mysten-infra", rev = "c1ab69f1a4004414bf0dee412f2e5839b71bc8f6" }
+
+[dev-dependencies]
+fdlimit = "0.2.1"
diff --git a/fastpay_core/src/authority.rs b/fastpay_core/src/authority.rs
@@ -17,7 +17,6 @@ use move_core_types::{
     resolver::{ModuleResolver, ResourceResolver},
 };
 use move_vm_runtime::native_functions::NativeFunctionTable;
-use std::path::Path;
 use std::{
     collections::{BTreeMap, HashSet},
     sync::Arc,
@@ -31,7 +30,7 @@ mod temporary_store;
 use temporary_store::AuthorityTemporaryStore;
 
 mod authority_store;
-use authority_store::AuthorityStore;
+pub use authority_store::AuthorityStore;
 
 pub struct AuthorityState {
     // Fixed size, static, identity of the authority
@@ -282,18 +281,18 @@ impl AuthorityState {
 }
 
 impl AuthorityState {
-    pub fn new<P: AsRef<Path>>(
+    pub fn new(
         committee: Committee,
         name: AuthorityName,
         secret: KeyPair,
-        path: P,
+        store: Arc<AuthorityStore>,
     ) -> Self {
         AuthorityState {
             committee,
             name,
             secret,
             native_functions: NativeFunctionTable::new(),
-            _database: Arc::new(AuthorityStore::open(path)),
+            _database: store,
         }
     }
 

diff --git a/fastpay_core/src/authority/authority_store.rs b/fastpay_core/src/authority/authority_store.rs
@@ -1,5 +1,6 @@
 use super::*;
 
+use rocksdb::Options;
 use std::path::Path;
 use std::sync::Mutex;
 use typed_store::rocks::{open_cf, DBMap};
@@ -16,10 +17,10 @@ pub struct AuthorityStore {
 
 impl AuthorityStore {
     /// Open an authority store by directory path
-    pub fn open<P: AsRef<Path>>(path: P) -> AuthorityStore {
+    pub fn open<P: AsRef<Path>>(path: P, db_options: Option<Options>) -> AuthorityStore {
         let db = open_cf(
             &path,
-            None,
+            db_options,
             &[
                 "objects",
                 "order_lock",

diff --git a/fastpay_core/src/unit_tests/authority_tests.rs b/fastpay_core/src/unit_tests/authority_tests.rs
@@ -12,8 +12,16 @@ use move_binary_format::{
 };
 use move_core_types::ident_str;
 
-use std::env;
 use std::fs;
+use std::{convert::TryInto, env};
+
+pub fn system_maxfiles() -> usize {
+    fdlimit::raise_fd_limit().unwrap_or(256u64) as usize
+}
+
+fn max_files_authority_tests() -> i32 {
+    (system_maxfiles() / 8).try_into().unwrap()
+}
 
 #[tokio::test]
 async fn test_handle_transfer_order_bad_signature() {
@@ -572,11 +580,14 @@ async fn test_authority_persist() {
     fs::create_dir(&path).unwrap();
 
     // Create an authority
+    let mut opts = rocksdb::Options::default();
+    opts.set_max_open_files(max_files_authority_tests());
+    let store = Arc::new(AuthorityStore::open(&path, Some(opts)));
     let authority = AuthorityState::new(
         committee.clone(),
         authority_address,
         authority_key.copy(),
-        &path,
+        store,
     );
 
     // Create an object
@@ -593,7 +604,10 @@ async fn test_authority_persist() {
     drop(authority);
 
     // Reopen the authority with the same path
-    let authority2 = AuthorityState::new(committee, authority_address, authority_key, &path);
+    let mut opts = rocksdb::Options::default();
+    opts.set_max_open_files(max_files_authority_tests());
+    let store = Arc::new(AuthorityStore::open(&path, Some(opts)));
+    let authority2 = AuthorityState::new(committee, authority_address, authority_key, store);
     let obj2 = authority2.object_state(&object_id).await.unwrap();
 
     // Check the object is present
@@ -618,7 +632,10 @@ fn init_state() -> AuthorityState {
     let path = dir.join(format!("DB_{:?}", ObjectID::random()));
     fs::create_dir(&path).unwrap();
 
-    AuthorityState::new(committee, authority_address, authority_key, path)
+    let mut opts = rocksdb::Options::default();
+    opts.set_max_open_files(max_files_authority_tests());
+    let store = Arc::new(AuthorityStore::open(path, Some(opts)));
+    AuthorityState::new(committee, authority_address, authority_key, store)
 }
 
 #[cfg(test)]

diff --git a/fastpay_core/src/unit_tests/client_tests.rs b/fastpay_core/src/unit_tests/client_tests.rs
@@ -3,18 +3,27 @@
 #![allow(clippy::same_item_push)] // get_key_pair returns random elements
 
 use super::*;
-use crate::authority::AuthorityState;
+use crate::authority::{AuthorityState, AuthorityStore};
 use fastx_types::object::Object;
 use futures::lock::Mutex;
 use std::{
     collections::{BTreeMap, HashMap},
+    convert::TryInto,
     sync::Arc,
 };
 use tokio::runtime::Runtime;
 
 use std::env;
 use std::fs;
 
+pub fn system_maxfiles() -> usize {
+    fdlimit::raise_fd_limit().unwrap_or(256u64) as usize
+}
+
+fn max_files_client_tests() -> i32 {
+    (system_maxfiles() / 8).try_into().unwrap()
+}
+
 #[derive(Clone)]
 struct LocalAuthorityClient(Arc<Mutex<AuthorityState>>);
 
@@ -73,7 +82,10 @@ fn init_local_authorities(
         let path = dir.join(format!("DB_{:?}", ObjectID::random()));
         fs::create_dir(&path).unwrap();
 
-        let state = AuthorityState::new(committee.clone(), address, secret, path);
+        let mut opts = rocksdb::Options::default();
+        opts.set_max_open_files(max_files_client_tests());
+        let store = Arc::new(AuthorityStore::open(path, Some(opts)));
+        let state = AuthorityState::new(committee.clone(), address, secret, store);
         clients.insert(address, LocalAuthorityClient::new(state));
     }
     (clients, committee)
@@ -104,7 +116,10 @@ fn init_local_authorities_bad_1(
         let path = dir.join(format!("DB_{:?}", ObjectID::random()));
         fs::create_dir(&path).unwrap();
 
-        let state = AuthorityState::new(committee.clone(), address, secret, path);
+        let mut opts = rocksdb::Options::default();
+        opts.set_max_open_files(max_files_client_tests());
+        let store = Arc::new(AuthorityStore::open(path, Some(opts)));
+        let state = AuthorityState::new(committee.clone(), address, secret, store);
         clients.insert(address, LocalAuthorityClient::new(state));
     }
     (clients, committee)
@@ -219,7 +234,6 @@ fn test_get_strong_majority_owner() {
 }
 
 #[test]
-#[ignore = "Enable after https://github.com/MystenLabs/fastnft/issues/109 is fixed"]
 fn test_initiating_valid_transfer() {
     let mut rt = Runtime::new().unwrap();
     let (recipient, _) = get_key_pair();
@@ -273,7 +287,6 @@ fn test_initiating_valid_transfer() {
 }
 
 #[test]
-#[ignore = "Enable after https://github.com/MystenLabs/fastnft/issues/109 is fixed"]
 fn test_initiating_valid_transfer_despite_bad_authority() {
     let mut rt = Runtime::new().unwrap();
     let (recipient, _) = get_key_pair();