-
Notifications
You must be signed in to change notification settings - Fork 12
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
RocksDB Storage service #584
Conversation
|
||
/// Rocks transaction type | ||
// Do not use `TransactionDB` here, because rocksdb's `TransactionDB` does not support open by read-only mode. | ||
// Thus, we cannot open the same db in two or more processes. |
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.
So, we cannot have a process writing and a process reading?
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.
We can because we use DB
here, not the TransactionDB
. rocksdb
crate has two kinds of db, DB
and TransactionDB
, TransactionDB
does not allow one read process while one write process. However, DB
supports.
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.
Looking good. Can we add a test where we open the same DB from 2 processes, one opens and writes the other reads?
I added a bin, so we can manually test it by using two processes, and there is also a unit test with one write thread, and multiple threads read. |
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.
Looking good. Fix CI and ahead with it!! Thanks!
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.
LGTM!
async fn test_store_load_remove( | ||
) -> Result<(), <RocksBackend<NoStorageSerde> as StorageBackend>::Error> { | ||
let temp_path = TempDir::new().unwrap(); | ||
let sled_settings = RocksBackendSettings { |
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.
let sled_settings = RocksBackendSettings { | |
let rocks_settings = RocksBackendSettings { |
also, in other tests
Implement RocksDB storage service, not use
TransactionDB
as it does not support open in read-only mode.