-
Notifications
You must be signed in to change notification settings - Fork 38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add rocksdb interface #29
Conversation
Milchstra3e
commented
Jul 31, 2022
- add some dependency
- add rocksdb_kv module
- implement rocksdb functions except revert_to_latest_checkpoint
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of using block_on
, make the test tokio::test
and just .await
it.
kv-storage/src/rocksdb_kv/mod.rs
Outdated
let result = self.db.put(key.as_ref(), value); | ||
match result { | ||
Ok(_) => Ok(()), | ||
Err(_) => Err(super::Error::Unknown("unknown error".to_string())), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use the err information, not just discarding it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix
kv-storage/src/rocksdb_kv/mod.rs
Outdated
let result = self.db.delete(key.as_ref()); | ||
match result { | ||
Ok(_) => Ok(()), | ||
Err(_) => Err(super::Error::NotFound), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use the err information, not just discarding it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix
kv-storage/src/rocksdb_kv/mod.rs
Outdated
let result = self.db.get(key.as_ref()); | ||
match result { | ||
Ok(v) => Ok(v.unwrap()), | ||
Err(_) => Err(super::Error::NotFound), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use the err information, not just discarding it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix
kv-storage/src/rocksdb_kv/mod.rs
Outdated
.unwrap(); | ||
} | ||
|
||
self.db = DB::open_default(new_current_db_dir.to_path_buf().as_path().join("db")).unwrap(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't unwrap, but handle it and propagate using ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix
kv-storage/src/rocksdb_kv/mod.rs
Outdated
.unwrap(); | ||
checkpoint_db | ||
.create_checkpoint(new_checkpoint_db_dir.to_path_buf().as_path().join("db")) | ||
.unwrap(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't unwrap, but handle it and propagate using ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix
kv-storage/src/rocksdb_kv/mod.rs
Outdated
.unwrap(); | ||
checkpoint_db | ||
.create_checkpoint(checkpoint_db_dir.to_path_buf().as_path().join("db")) | ||
.unwrap(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't unwrap, but handle it and propagate using ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix
kv-storage/src/rocksdb_kv/mod.rs
Outdated
|
||
checkpoint_db | ||
.create_checkpoint(current_db_dir.to_path_buf().as_path().join("db")) | ||
.unwrap(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't unwrap, but handle it and propagate using ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix
kv-storage/src/rocksdb_kv/mod.rs
Outdated
.unwrap(); | ||
checkpoint_db | ||
.create_checkpoint(checkpoint_db_dir.to_path_buf().as_path().join("db")) | ||
.unwrap(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't unwrap, but handle it and propagate using ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix
kv-storage/src/rocksdb_kv/mod.rs
Outdated
} | ||
|
||
Ok(RocksDB { | ||
db: DB::open_default(current_db_dir.to_path_buf().as_path().join("db")).unwrap(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't unwrap, but handle it and propagate using ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix
kv-storage/src/rocksdb_kv/mod.rs
Outdated
db.put(Hash256::hash("key3"), "val3").unwrap(); | ||
db.put(Hash256::hash("key4"), "val4").unwrap(); | ||
|
||
tmp_folder |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tmp_directory
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix
add new folder(rocksdb) in kv-store init RocksDB struct without fields and implementation each functions
change folder name from rocksdb_kv to rocksdb change file name from rocksdb.rs to mod.rs
include rocksdb crate declare new field db in RocksDB struct implement function new / open / insert_or_update / remove / get
add new dependency mktemp use mktemp module implement comit_checkpoint
- add origin_path: PathBuf - add current_db_path: Temp - add checkpoint_Db_path: Temp - rewrite logic of new / open
kv-storage/src/rocksdb_kv/mod.rs
Outdated
} | ||
|
||
async fn contain(&self, key: Hash256) -> Result<bool, super::Error> { | ||
let result = self.db.get(key.as_ref()).unwrap(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please handle the error instead of unwrapping it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix
kv-storage/src/rocksdb_kv/mod.rs
Outdated
let result = self.db.delete(key.as_ref()); | ||
match result { | ||
Ok(_) => Ok(()), | ||
Err(e) => Err(super::Error::from(e)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this? Not just ?
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix