From 0fc2441a2eb12f8c575aa9b92efe2875627d76b9 Mon Sep 17 00:00:00 2001 From: Tyler Neely Date: Sat, 7 Nov 2015 16:19:49 -0800 Subject: [PATCH 1/3] nuke DBResult --- src/lib.rs | 2 +- src/main.rs | 54 ++++++------ src/merge_operator.rs | 25 +++--- src/rocksdb.rs | 106 ++++-------------------- test/test_column_family.rs | 23 +++--- test/test_column_family.rs.orig | 141 ++++++++++++++++++++++++++++++++ test/test_multithreaded.rs | 6 +- test/test_multithreaded.rs.orig | 56 +++++++++++++ 8 files changed, 274 insertions(+), 139 deletions(-) create mode 100644 test/test_column_family.rs.orig create mode 100644 test/test_multithreaded.rs.orig diff --git a/src/lib.rs b/src/lib.rs index 9ba104a..eb1f9e5 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -18,7 +18,7 @@ pub use ffi as rocksdb_ffi; pub use ffi::{new_bloom_filter, DBCompactionStyle, DBComparator}; -pub use rocksdb::{DB, DBResult, DBVector, WriteBatch, Writable, Direction}; +pub use rocksdb::{DB, DBVector, WriteBatch, Writable, Direction}; pub use rocksdb_options::{Options, BlockBasedOptions}; pub use merge_operator::MergeOperands; pub mod rocksdb; diff --git a/src/main.rs b/src/main.rs index 4c4eabb..42abc58 100644 --- a/src/main.rs +++ b/src/main.rs @@ -49,16 +49,18 @@ fn main() { let path = "/tmp/rust-rocksdb"; let mut db = DB::open_default(path).unwrap(); assert!(db.put(b"my key", b"my value").is_ok()); - db.get(b"my key").map( |value| { + match db.get(b"my key") { + Ok(Some(value)) => { match value.to_utf8() { - Some(v) => - println!("retrieved utf8 value: {}", v), - None => - println!("did not read valid utf-8 out of the db"), + Some(v) => + println!("retrieved utf8 value: {}", v), + None => + println!("did not read valid utf-8 out of the db"), } - }) - .on_absent( || { println!("value not found") }) - .on_error( |e| { println!("error retrieving value: {}", e) }); + }, + Err(e) => println!("error retrieving value: {}", e), + _ => panic!("value not present!"), + } assert!(db.delete(b"my key").is_ok()); @@ -88,24 +90,25 @@ fn custom_merge() { opts.create_if_missing(true); opts.add_merge_operator("test operator", concat_merge); { - let mut db = DB::open(&opts, path).unwrap(); - db.put(b"k1", b"a"); - db.merge(b"k1", b"b"); - db.merge(b"k1", b"c"); - db.merge(b"k1", b"d"); - db.merge(b"k1", b"efg"); - db.merge(b"k1", b"h"); - db.get(b"k1").map( |value| { + let db = DB::open(&opts, path).unwrap(); + db.put(b"k1", b"a").unwrap(); + db.merge(b"k1", b"b").unwrap(); + db.merge(b"k1", b"c").unwrap(); + db.merge(b"k1", b"d").unwrap(); + db.merge(b"k1", b"efg").unwrap(); + db.merge(b"k1", b"h").unwrap(); + match db.get(b"k1") { + Ok(Some(value)) => { match value.to_utf8() { - Some(v) => - println!("retrieved utf8 value: {}", v), - None => - println!("did not read valid utf-8 out of the db"), + Some(v) => + println!("retrieved utf8 value: {}", v), + None => + println!("did not read valid utf-8 out of the db"), } - }) - .on_absent( || { println!("value not found") }) - .on_error( |e| { println!("error retrieving value: {}", e) }); - + } + Err(e) => println!("error retrieving value: {}", e), + _ => panic!("value not present!"), + } } DB::destroy(&opts, path).is_ok(); } @@ -130,8 +133,7 @@ fn main() { None => panic!("value corrupted"), } }) - .on_absent( || { panic!("value not found") }) - .on_error( |e| { panic!("error retrieving value: {}", e) }); + .or_else( |e| { panic!("error retrieving value: {}", e) }); db.delete(b"k1"); } } diff --git a/src/merge_operator.rs b/src/merge_operator.rs index 4495380..4b863aa 100644 --- a/src/merge_operator.rs +++ b/src/merge_operator.rs @@ -21,7 +21,7 @@ use std::ptr; use std::slice; use rocksdb_options::Options; -use rocksdb::{DB, DBResult, DBVector, Writable}; +use rocksdb::{DB, DBVector, Writable}; pub struct MergeOperatorCallback { pub name: CString, @@ -196,21 +196,24 @@ fn mergetest() { db.merge(b"k1", b"efg"); let m = db.merge(b"k1", b"h"); assert!(m.is_ok()); - db.get(b"k1").map( |value| { + match db.get(b"k1") { + Ok(Some(value)) => { match value.to_utf8() { - Some(v) => - println!("retrieved utf8 value: {}", v), - None => - println!("did not read valid utf-8 out of the db"), + Some(v) => + println!("retrieved utf8 value: {}", v), + None => + println!("did not read valid utf-8 out of the db"), } - }).on_absent( || { println!("value not present!") }) - .on_error( |e| { println!("error reading value")}); //: {", e) }); + }, + Err(e) => { println!("error reading value")}, + _ => panic!("value not present"), + } assert!(m.is_ok()); - let r: DBResult = db.get(b"k1"); - assert!(r.unwrap().to_utf8().unwrap() == "abcdefgh"); + let r: Result, String> = db.get(b"k1"); + assert!(r.unwrap().unwrap().to_utf8().unwrap() == "abcdefgh"); assert!(db.delete(b"k1").is_ok()); - assert!(db.get(b"k1").is_none()); + assert!(db.get(b"k1").unwrap().is_none()); } assert!(DB::destroy(&opts, path).is_ok()); } diff --git a/src/rocksdb.rs b/src/rocksdb.rs index 4150c31..0a06fc1 100644 --- a/src/rocksdb.rs +++ b/src/rocksdb.rs @@ -327,11 +327,11 @@ impl DB { return Ok(()) } - pub fn get(&self, key: &[u8]) -> DBResult { + pub fn get(&self, key: &[u8]) -> Result, String> { unsafe { let readopts = rocksdb_ffi::rocksdb_readoptions_create(); if readopts.0.is_null() { - return DBResult::Error("Unable to create rocksdb read \ + return Err("Unable to create rocksdb read \ options. This is a fairly trivial call, and its failure \ may be indicative of a mis-compiled or mis-loaded rocksdb \ library.".to_string()); @@ -345,22 +345,22 @@ impl DB { key.as_ptr(), key.len() as size_t, val_len_ptr, err_ptr) as *mut u8; rocksdb_ffi::rocksdb_readoptions_destroy(readopts); if !err.is_null() { - return DBResult::Error(error_message(err)); + return Err(error_message(err)); } match val.is_null() { - true => DBResult::None, + true => Ok(None), false => { - DBResult::Some(DBVector::from_c(val, val_len)) + Ok(Some(DBVector::from_c(val, val_len))) } } } } - pub fn get_cf(&self, cf: DBCFHandle, key: &[u8]) -> DBResult { + pub fn get_cf(&self, cf: DBCFHandle, key: &[u8]) -> Result, String> { unsafe { let readopts = rocksdb_ffi::rocksdb_readoptions_create(); if readopts.0.is_null() { - return DBResult::Error("Unable to create rocksdb read \ + return Err("Unable to create rocksdb read \ options. This is a fairly trivial call, and its failure \ may be indicative of a mis-compiled or mis-loaded rocksdb \ library.".to_string()); @@ -375,12 +375,12 @@ impl DB { err_ptr) as *mut u8; rocksdb_ffi::rocksdb_readoptions_destroy(readopts); if !err.is_null() { - return DBResult::Error(error_message(err)); + return Err(error_message(err)); } match val.is_null() { - true => DBResult::None, + true => Ok(None), false => { - DBResult::Some(DBVector::from_c(val, val_len)) + Ok(Some(DBVector::from_c(val, val_len))) } } } @@ -691,76 +691,6 @@ impl DBVector { } } -// DBResult exists because of the inherent difference between -// an operational failure and the absence of a possible result. -#[derive(Clone, PartialEq, PartialOrd, Eq, Ord, Debug)] -pub enum DBResult { - Some(T), - None, - Error(E), -} - -impl DBResult { - pub fn map U>(self, f: F) -> DBResult { - match self { - DBResult::Some(x) => DBResult::Some(f(x)), - DBResult::None => DBResult::None, - DBResult::Error(e) => DBResult::Error(e), - } - } - - pub fn unwrap(self) -> T { - match self { - DBResult::Some(x) => x, - DBResult::None => - panic!("Attempted unwrap on DBResult::None"), - DBResult::Error(_) => - panic!("Attempted unwrap on DBResult::Error"), - } - } - - pub fn on_error U>(self, f: F) -> DBResult { - match self { - DBResult::Some(x) => DBResult::Some(x), - DBResult::None => DBResult::None, - DBResult::Error(e) => DBResult::Error(f(e)), - } - } - - pub fn on_absent ()>(self, f: F) -> DBResult { - match self { - DBResult::Some(x) => DBResult::Some(x), - DBResult::None => { - f(); - DBResult::None - }, - DBResult::Error(e) => DBResult::Error(e), - } - } - - pub fn is_some(self) -> bool { - match self { - DBResult::Some(_) => true, - DBResult::None => false, - DBResult::Error(_) => false, - } - } - pub fn is_none(self) -> bool { - match self { - DBResult::Some(_) => false, - DBResult::None => true, - DBResult::Error(_) => false, - } - } - pub fn is_error(self) -> bool { - match self { - DBResult::Some(_) => false, - DBResult::None => false, - DBResult::Error(_) => true, - } - } -} - #[test] fn external() { let path = "_rust_rocksdb_externaltest"; @@ -768,10 +698,10 @@ fn external() { let mut db = DB::open_default(path).unwrap(); let p = db.put(b"k1", b"v1111"); assert!(p.is_ok()); - let r: DBResult = db.get(b"k1"); - assert!(r.unwrap().to_utf8().unwrap() == "v1111"); + let r: Result, String> = db.get(b"k1"); + assert!(r.unwrap().unwrap().to_utf8().unwrap() == "v1111"); assert!(db.delete(b"k1").is_ok()); - assert!(db.get(b"k1").is_none()); + assert!(db.get(b"k1").unwrap().is_none()); } let opts = Options::new(); let result = DB::destroy(&opts, path); @@ -797,20 +727,20 @@ fn writebatch_works() { let mut db = DB::open_default(path).unwrap(); { // test put let mut batch = WriteBatch::new(); - assert!(db.get(b"k1").is_none()); + assert!(db.get(b"k1").unwrap().is_none()); batch.put(b"k1", b"v1111"); - assert!(db.get(b"k1").is_none()); + assert!(db.get(b"k1").unwrap().is_none()); let p = db.write(batch); assert!(p.is_ok()); - let r: DBResult = db.get(b"k1"); - assert!(r.unwrap().to_utf8().unwrap() == "v1111"); + let r: Result, String> = db.get(b"k1"); + assert!(r.unwrap().unwrap().to_utf8().unwrap() == "v1111"); } { // test delete let mut batch = WriteBatch::new(); batch.delete(b"k1"); let p = db.write(batch); assert!(p.is_ok()); - assert!(db.get(b"k1").is_none()); + assert!(db.get(b"k1").unwrap().is_none()); } } let opts = Options::new(); diff --git a/test/test_column_family.rs b/test/test_column_family.rs index 0a20354..d779110 100644 --- a/test/test_column_family.rs +++ b/test/test_column_family.rs @@ -13,7 +13,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -use rocksdb::{Options, DB, DBResult, Writable, Direction, MergeOperands}; +use rocksdb::{Options, DB, Writable, Direction, MergeOperands}; #[test] pub fn test_column_family() { @@ -67,7 +67,7 @@ pub fn test_column_family() { }; let cf1 = *db.cf_handle("cf1").unwrap(); assert!(db.put_cf(cf1, b"k1", b"v1").is_ok()); - assert!(db.get_cf(cf1, b"k1").unwrap().to_utf8().unwrap() == "v1"); + assert!(db.get_cf(cf1, b"k1").unwrap().unwrap().to_utf8().unwrap() == "v1"); let p = db.put_cf(cf1, b"k1", b"a"); assert!(p.is_ok()); db.merge_cf(cf1, b"k1", b"b"); @@ -77,20 +77,23 @@ pub fn test_column_family() { let m = db.merge_cf(cf1, b"k1", b"h"); println!("m is {:?}", m); // TODO assert!(m.is_ok()); - db.get(b"k1").map( |value| { + match db.get(b"k1") { + Ok(Some(value)) => { match value.to_utf8() { - Some(v) => - println!("retrieved utf8 value: {}", v), - None => - println!("did not read valid utf-8 out of the db"), + Some(v) => + println!("retrieved utf8 value: {}", v), + None => + println!("did not read valid utf-8 out of the db"), } - }).on_absent( || { println!("value not present!") }) - .on_error( |e| { println!("error reading value")}); //: {", e) }); + }, + Err(e) => println!("error reading value"), + _ => panic!("value not present!"), + } let r = db.get_cf(cf1, b"k1"); // TODO assert!(r.unwrap().to_utf8().unwrap() == "abcdefgh"); assert!(db.delete(b"k1").is_ok()); - assert!(db.get(b"k1").is_none()); + assert!(db.get(b"k1").unwrap().is_none()); } // TODO should be able to use writebatch ops with a cf { diff --git a/test/test_column_family.rs.orig b/test/test_column_family.rs.orig new file mode 100644 index 0000000..789ec80 --- /dev/null +++ b/test/test_column_family.rs.orig @@ -0,0 +1,141 @@ +/* + Copyright 2014 Tyler Neely + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ +use rocksdb::{Options, DB, DBResult, Writable, Direction, MergeOperands}; + +#[test] +pub fn test_column_family() { + let path = "_rust_rocksdb_cftest"; + + // should be able to create column families + { + let mut opts = Options::new(); + opts.create_if_missing(true); + opts.add_merge_operator("test operator", test_provided_merge); + let mut db = DB::open(&opts, path).unwrap(); + let opts = Options::new(); + match db.create_cf("cf1", &opts) { + Ok(_) => println!("cf1 created successfully"), + Err(e) => { + panic!("could not create column family: {}", e); + }, + } + } + + // should fail to open db without specifying same column families + { + let mut opts = Options::new(); + opts.add_merge_operator("test operator", test_provided_merge); + match DB::open(&opts, path) { + Ok(_) => panic!("should not have opened DB successfully without specifying column + families"), + Err(e) => assert!(e.starts_with("Invalid argument: You have to open all column families.")), + } + } + + // should properly open db when specyfing all column families + { + let mut opts = Options::new(); + opts.add_merge_operator("test operator", test_provided_merge); + match DB::open_cf(&opts, path, &["cf1"]) { + Ok(_) => println!("successfully opened db with column family"), + Err(e) => panic!("failed to open db with column family: {}", e), + } + } + // TODO should be able to write, read, merge, batch, and iterate over a cf + { + let mut opts = Options::new(); + opts.add_merge_operator("test operator", test_provided_merge); + let mut db = match DB::open_cf(&opts, path, &["cf1"]) { + Ok(db) => { + println!("successfully opened db with column family"); + db + }, + Err(e) => panic!("failed to open db with column family: {}", e), + }; + let cf1 = *db.cf_handle("cf1").unwrap(); + assert!(db.put_cf(cf1, b"k1", b"v1").is_ok()); + assert!(db.get_cf(cf1, b"k1").unwrap().unwrap().to_utf8().unwrap() == "v1"); + let p = db.put_cf(cf1, b"k1", b"a"); + assert!(p.is_ok()); + db.merge_cf(cf1, b"k1", b"b"); + db.merge_cf(cf1, b"k1", b"c"); + db.merge_cf(cf1, b"k1", b"d"); + db.merge_cf(cf1, b"k1", b"efg"); + let m = db.merge_cf(cf1, b"k1", b"h"); + println!("m is {:?}", m); + // TODO assert!(m.is_ok()); + match db.get(b"k1") { + Ok(Some(value)) => { + match value.to_utf8() { + Some(v) => + println!("retrieved utf8 value: {}", v), + None => + println!("did not read valid utf-8 out of the db"), + } +<<<<<<< Updated upstream + }).on_absent( || { println!("value not present!") }) + .on_error( |e| { println!("error reading value")}); //: {", e) }); +======= + }, + Err(e) => println!("error reading value"), + _ => panic!("value not present!"), + } +>>>>>>> Stashed changes + + let r = db.get_cf(cf1, b"k1"); + // TODO assert!(r.unwrap().to_utf8().unwrap() == "abcdefgh"); + assert!(db.delete(b"k1").is_ok()); + assert!(db.get(b"k1").unwrap().is_none()); + } + // TODO should be able to use writebatch ops with a cf + { + } + // TODO should be able to iterate over a cf + { + } + // should b able to drop a cf + { + let mut db = DB::open_cf(&Options::new(), path, &["cf1"]).unwrap(); + match db.drop_cf("cf1") { + Ok(_) => println!("cf1 successfully dropped."), + Err(e) => panic!("failed to drop column family: {}", e), + } + } + + assert!(DB::destroy(&Options::new(), path).is_ok()); +} + +fn test_provided_merge(new_key: &[u8], + existing_val: Option<&[u8]>, + mut operands: &mut MergeOperands) + -> Vec { + let nops = operands.size_hint().0; + let mut result: Vec = Vec::with_capacity(nops); + match existing_val { + Some(v) => { + for e in v { + result.push(*e); + } + }, + None => (), + } + for op in operands { + for e in op { + result.push(*e); + } + } + result +} diff --git a/test/test_multithreaded.rs b/test/test_multithreaded.rs index 674b29d..32f00f4 100644 --- a/test/test_multithreaded.rs +++ b/test/test_multithreaded.rs @@ -1,5 +1,5 @@ -use rocksdb::{Options, DB, Writable, Direction, DBResult}; -use std::thread::{self, Builder}; +use rocksdb::{Options, DB, Writable}; +use std::thread; use std::sync::Arc; const N: usize = 100_000; @@ -31,7 +31,7 @@ pub fn test_multithreaded() { let j3 = thread::spawn(move|| { for i in 1..N { match db3.get(b"key") { - DBResult::Some(v) => { + Ok(Some(v)) => { if &v[..] != b"value1" && &v[..] != b"value2" { assert!(false); } diff --git a/test/test_multithreaded.rs.orig b/test/test_multithreaded.rs.orig new file mode 100644 index 0000000..141ed2e --- /dev/null +++ b/test/test_multithreaded.rs.orig @@ -0,0 +1,56 @@ +<<<<<<< Updated upstream +use rocksdb::{Options, DB, Writable, Direction, DBResult}; +use std::thread::{self, Builder}; +======= +use rocksdb::{Options, DB, Writable}; +use std::thread; +>>>>>>> Stashed changes +use std::sync::Arc; + +const N: usize = 100_000; + +#[test] +pub fn test_multithreaded() { + let path = "_rust_rocksdb_multithreadtest"; + { + let db = DB::open_default(path).unwrap(); + let db = Arc::new(db); + + db.put(b"key", b"value1"); + + let db1 = db.clone(); + let j1 = thread::spawn(move|| { + for i in 1..N { + db1.put(b"key", b"value1"); + } + }); + + let db2 = db.clone(); + let j2 = thread::spawn(move|| { + for i in 1..N { + db2.put(b"key", b"value2"); + } + }); + + let db3 = db.clone(); + let j3 = thread::spawn(move|| { + for i in 1..N { + match db3.get(b"key") { + Ok(Some(v)) => { + if &v[..] != b"value1" && &v[..] != b"value2" { + assert!(false); + } + } + _ => { + assert!(false); + } + } + } + }); + + j1.join(); + j2.join(); + j3.join(); + } + assert!(DB::destroy(&Options::new(), path).is_ok()); +} From 064ddec8ea1f57c0394d23cca33833d3a88d1602 Mon Sep 17 00:00:00 2001 From: Tyler Neely Date: Sat, 7 Nov 2015 16:24:31 -0800 Subject: [PATCH 2/3] never again... --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 8a4f43d..4fc9764 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,4 @@ *.swp target Cargo.lock +*.orig From 705420aa692563aa795904dc67e5aab5e1344233 Mon Sep 17 00:00:00 2001 From: Tyler Neely Date: Sat, 7 Nov 2015 16:30:03 -0800 Subject: [PATCH 3/3] Update readme, bump version. --- Cargo.toml | 2 +- README.md | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 894a6f7..bed6a80 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -2,7 +2,7 @@ name = "rocksdb" description = "A Rust wrapper for Facebook's RocksDB embeddable database." -version = "0.1.1" +version = "0.2.0" authors = ["Tyler Neely ", "David Greenberg "] license = "Apache-2.0" keywords = ["database", "embedded", "LSM-tree", "persistence"] diff --git a/README.md b/README.md index baa2f63..4d4bd49 100644 --- a/README.md +++ b/README.md @@ -3,7 +3,7 @@ rust-rocksdb [![Build Status](https://travis-ci.org/spacejam/rust-rocksdb.svg?branch=master)](https://travis-ci.org/spacejam/rust-rocksdb) [![crates.io](http://meritbadge.herokuapp.com/rocksdb)](https://crates.io/crates/rocksdb) -This library has been tested against RocksDB 3.13.1 on linux and OSX. The 0.1.1 crate should work with the Rust 1.2 stable and nightly releases as of 9/7/15. +This library has been tested against RocksDB 3.13.1 on linux and OSX. The 0.2.0 crate should work with the Rust 1.4 stable and nightly releases as of 11/7/15. ### status - [x] basic open/put/get/delete/close @@ -16,6 +16,7 @@ This library has been tested against RocksDB 3.13.1 on linux and OSX. The 0.1.1 - [x] comparator - [x] snapshot - [x] column family operations + - [ ] prefix seek - [ ] slicetransform - [ ] windows support @@ -35,7 +36,7 @@ sudo make install ###### Cargo.toml ```rust [dependencies] -rocksdb = "~0.1.1" +rocksdb = "~0.2.0" ``` ###### Code ```rust @@ -45,12 +46,11 @@ use rocksdb::{DB, Writable}; fn main() { let mut db = DB::open_default("/path/for/rocksdb/storage").unwrap(); db.put(b"my key", b"my value"); - db.get(b"my key") - .map( |value| { - println!("retrieved value {}", value.to_utf8().unwrap()) - }) - .on_absent( || { println!("value not found") }) - .on_error( |e| { println!("operational problem encountered: {}", e) }); + match db.get(b"my key") { + Ok(Some(value)) => println!("retrieved value {}", value.to_utf8().unwrap()), + Ok(None) => println!("value not found"), + Err(e) => println!("operational problem encountered: {}", e), + } db.delete(b"my key"); } @@ -141,7 +141,7 @@ fn main() { db.merge(b"k1", b"d"); db.merge(b"k1", b"efg"); let r = db.get(b"k1"); - assert!(r.unwrap().to_utf8().unwrap() == "abcdefg"); + assert!(r.unwrap().unwrap().to_utf8().unwrap() == "abcdefg"); } ```