Merge pull request #1202 from holochain/pickle-db-bugs

Fixing pickledb bugs
holochain · Apr 3, 2019 · 40521bb · 40521bb
2 parents e64541d + 455f3cd
commit 40521bb
Show file tree

Hide file tree

Showing 8 changed files with 79 additions and 34 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -26,6 +26,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Removes JsonString::From<String> and replaces it with JsonString::from_json(&str). This makes conversions more explicit and allows for validating that the string is actually valid json [#1184](https://github.com/holochain/holochain-rust/pull/1184)
 
 ### Fixed
+-This pull request fixes the various issues with the pickledb implementation. Better guards and directory fixes [#1202]
+(https://github.com/holochain/holochain-rust/pull/1202)
 
 ### Security
 

diff --git a/app_spec/test/test.js b/app_spec/test/test.js
@@ -20,13 +20,11 @@ scenario2.runTape('sign_and_verify_message', async (t, { alice, bob }) => {
     const message = "Hello everyone! Time to start the secret meeting";
 
     const SignResult = bob.call("converse", "sign_message", { key_id:"", message: message });
-
     t.deepEqual(SignResult, { Ok: 'YVystBCmNEJGW/91bg43cUUybbtiElex0B+QWYy+PlB+nE3W8TThYGE4QzuUexvzkGqSutV04dSN8oyZxTJiBg==' });
 
     const provenance = [bob.agentId, SignResult.Ok];
 
     const VerificationResult = alice.call("converse", "verify_message", { message, provenance });
-
     t.deepEqual(VerificationResult, { Ok: true });
 })
 

diff --git a/benchmarks/benches/my_benchmark.rs b/benchmarks/benches/my_benchmark.rs
@@ -6,8 +6,28 @@ extern crate tempfile;
 
 use self::tempfile::tempdir;
 use bencher::Bencher;
-use holochain_cas_implementations::eav::{file::EavFileStorage, pickle::EavPickleStorage};
+use holochain_cas_implementations::eav::{
+    file::EavFileStorage, memory::EavMemoryStorage, pickle::EavPickleStorage,
+};
 use holochain_core_types::cas::{content::ExampleAddressableContent, storage::EavTestSuite};
+
+fn bench_memory_eav_one_to_many(b: &mut Bencher) {
+    b.iter(|| {
+        let eav_storage = EavMemoryStorage::new();
+        EavTestSuite::test_one_to_many::<ExampleAddressableContent, EavMemoryStorage>(
+            eav_storage.clone(),
+        )
+    })
+}
+
+fn bench_memory_eav_many_to_one(b: &mut Bencher) {
+    b.iter(|| {
+        let eav_storage = EavMemoryStorage::new();
+        EavTestSuite::test_one_to_many::<ExampleAddressableContent, EavMemoryStorage>(
+            eav_storage.clone(),
+        )
+    })
+}
 fn bench_file_eav_one_to_many(b: &mut Bencher) {
     b.iter(|| {
         let temp = tempdir().expect("test was supposed to create temp dir");
@@ -54,6 +74,8 @@ fn bench_pickle_eav_many_to_one(b: &mut Bencher) {
 
 benchmark_group!(
     benches,
+    bench_memory_eav_many_to_one,
+    bench_memory_eav_one_to_many,
     bench_file_eav_one_to_many,
     bench_file_eav_many_to_one,
     bench_pickle_eav_many_to_one,

diff --git a/cas_implementations/src/cas/pickle.rs b/cas_implementations/src/cas/pickle.rs
@@ -31,21 +31,31 @@ impl Debug for PickleStorage {
 }
 
 impl PickleStorage {
-    pub fn new<P: AsRef<Path>>(db_path: P) -> PickleStorage {
+    pub fn new<P: AsRef<Path> + Clone>(db_path: P) -> PickleStorage {
+        let cas_db = db_path.as_ref().join("cas").with_extension("db");
         PickleStorage {
             id: Uuid::new_v4(),
-            db: Arc::new(RwLock::new(PickleDb::new(
-                db_path,
-                PickleDbDumpPolicy::PeriodicDump(PERSISTENCE_INTERVAL),
-                SerializationMethod::Cbor,
-            ))),
+            db: Arc::new(RwLock::new(
+                PickleDb::load(
+                    cas_db.clone(),
+                    PickleDbDumpPolicy::PeriodicDump(PERSISTENCE_INTERVAL),
+                    SerializationMethod::Cbor,
+                )
+                .unwrap_or_else(|_| {
+                    PickleDb::new(
+                        cas_db,
+                        PickleDbDumpPolicy::PeriodicDump(PERSISTENCE_INTERVAL),
+                        SerializationMethod::Cbor,
+                    )
+                }),
+            )),
         }
     }
 }
 
 impl ContentAddressableStorage for PickleStorage {
     fn add(&mut self, content: &AddressableContent) -> Result<(), HolochainError> {
-        let mut inner = self.db.write()?;
+        let mut inner = self.db.write().unwrap();
 
         inner
             .set(&content.address().to_string(), &content.content())
@@ -55,13 +65,13 @@ impl ContentAddressableStorage for PickleStorage {
     }
 
     fn contains(&self, address: &Address) -> Result<bool, HolochainError> {
-        let inner = self.db.read()?;
+        let inner = self.db.read().unwrap();
 
         Ok(inner.exists(&address.to_string()))
     }
 
     fn fetch(&self, address: &Address) -> Result<Option<Content>, HolochainError> {
-        let inner = self.db.read()?;
+        let inner = self.db.read().unwrap();
 
         Ok(inner.get(&address.to_string()))
     }

diff --git a/cas_implementations/src/eav/pickle.rs b/cas_implementations/src/eav/pickle.rs
@@ -21,14 +21,24 @@ pub struct EavPickleStorage {
 }
 
 impl EavPickleStorage {
-    pub fn new<P: AsRef<Path>>(db_path: P) -> EavPickleStorage {
+    pub fn new<P: AsRef<Path> + Clone>(db_path: P) -> EavPickleStorage {
+        let eav_db = db_path.as_ref().join("eav").with_extension("db");
         EavPickleStorage {
             id: Uuid::new_v4(),
-            db: Arc::new(RwLock::new(PickleDb::new(
-                db_path,
-                PickleDbDumpPolicy::PeriodicDump(PERSISTENCE_INTERVAL),
-                SerializationMethod::Cbor,
-            ))),
+            db: Arc::new(RwLock::new(
+                PickleDb::load(
+                    eav_db.clone(),
+                    PickleDbDumpPolicy::PeriodicDump(PERSISTENCE_INTERVAL),
+                    SerializationMethod::Cbor,
+                )
+                .unwrap_or_else(|_| {
+                    PickleDb::new(
+                        eav_db,
+                        PickleDbDumpPolicy::PeriodicDump(PERSISTENCE_INTERVAL),
+                        SerializationMethod::Cbor,
+                    )
+                }),
+            )),
         }
     }
 }
@@ -46,19 +56,22 @@ impl EntityAttributeValueStorage for EavPickleStorage {
         &mut self,
         eav: &EntityAttributeValueIndex,
     ) -> Result<Option<EntityAttributeValueIndex>, HolochainError> {
-        let index_str = eav.index().to_string();
-        let value = self.db.read()?.get::<EntityAttributeValueIndex>(&index_str);
-        if let Some(_) = value {
-            let new_eav =
+        let mut inner = self.db.write().unwrap();
+
+        //hate to introduce mutability but it is saved by the immutable clones at the end
+        let mut index_str = eav.index().to_string();
+        let mut value = inner.get::<EntityAttributeValueIndex>(&index_str);
+        let mut new_eav = eav.clone();
+        while value.is_some() {
+            new_eav =
                 EntityAttributeValueIndex::new(&eav.entity(), &eav.attribute(), &eav.value())?;
-            self.add_eavi(&new_eav)
-        } else {
-            self.db
-                .write()?
-                .set(&*index_str, &eav)
-                .map_err(|e| HolochainError::ErrorGeneric(e.to_string()))?;
-            Ok(Some(eav.clone()))
+            index_str = new_eav.index().to_string();
+            value = inner.get::<EntityAttributeValueIndex>(&index_str);
         }
+        inner
+            .set(&*index_str, &new_eav)
+            .map_err(|e| HolochainError::ErrorGeneric(e.to_string()))?;
+        Ok(Some(new_eav.clone()))
     }
 
     fn fetch_eavi(
@@ -74,7 +87,6 @@ impl EntityAttributeValueStorage for EavPickleStorage {
             .filter(|filter| filter.is_some())
             .map(|y| y.unwrap())
             .collect::<BTreeSet<EntityAttributeValueIndex>>();
-
         let entries_iter = entries.iter().cloned();
         Ok(query.run(entries_iter))
     }

diff --git a/conductor_api/src/conductor/admin.rs b/conductor_api/src/conductor/admin.rs
@@ -189,7 +189,7 @@ impl ConductorAdmin for Conductor {
             id: id.to_string(),
             dna: dna_id.to_string(),
             agent: agent_id.to_string(),
-            storage: StorageConfiguration::File {
+            storage: StorageConfiguration::Pickle {
                 path: storage_path
                     .to_str()
                     .ok_or(HolochainError::ConfigError(
@@ -1038,7 +1038,7 @@ id = 'new-instance'"#,
         toml = add_block(
             toml,
             format!(
-                "[instances.storage]\npath = '{}'\ntype = 'file'",
+                "[instances.storage]\npath = '{}'\ntype = 'pickle'",
                 storage_path_string
             ),
         );
@@ -1344,7 +1344,7 @@ id = 'new-instance-2'"#,
         toml = add_block(
             toml,
             format!(
-                "[instances.storage]\npath = '{}'\ntype = 'file'",
+                "[instances.storage]\npath = '{}'\ntype = 'pickle'",
                 storage_path_string
             ),
         );

diff --git a/nodejs_conductor/native/Cargo.toml b/nodejs_conductor/native/Cargo.toml
@@ -24,3 +24,4 @@ holochain_core_types = { path = "../../core_types" }
 holochain_node_test_waiter = { path = "../../nodejs_waiter" }
 neon-serde = "=0.1.1"
 rust_sodium-sys = "=0.10.3"
+
diff --git a/nodejs_conductor/native/src/config.rs b/nodejs_conductor/native/src/config.rs
@@ -87,7 +87,7 @@ fn make_config(instance_data: Vec<InstanceData>, logger: LoggerConfiguration) ->
             id: instance.name,
             agent: agent_id,
             dna: dna_id,
-            storage: StorageConfiguration::Memory,
+            storage: StorageConfiguration::Memory
         };
         instance_configs.push(instance);
     }