-
Notifications
You must be signed in to change notification settings - Fork 718
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
Support RocksDB transaction. #565
Conversation
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.
nice work after initial quick glance! it's been some time since i missed this functionality. thanks for contributing.
wow great progress! i'll review this in depth later |
merge two-phase commit implementation.
Resolve conflicts please. |
# Conflicts: # src/backup.rs # src/db.rs # tests/test_db.rs
fix: optimistic db write batch
Make DBCommon suitable for checkpointing
# Conflicts: # src/backup.rs # src/checkpoint.rs # src/db.rs # src/db_iterator.rs
@yiyuanliu do you plan to finish this PR? |
I will finish it, but it will take some time. Due to a job change, I've left the project I was working on when I built this PR. I'm sorry I haven't been able to find the time to work on this PR for a while. |
# Conflicts: # src/backup.rs # src/checkpoint.rs # src/db.rs # src/db_iterator.rs
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 to me, though I would admit, that I can spend more time checking the pull request if I have more time.
// | ||
|
||
use crate::ffi; | ||
use core::panic; |
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.
Why is the core::panic
import needed?
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.
I'll remove it later but for now, I merge it as is.
We are building a project which uses RocksDB transaction. However,
rust-rocksdb
doesn't support transaction. PR #250 has not been updated in a long time and does not appear to be maintained.This PR wants to provide support for transactions with minimal impact on existing code. I have finished wrapping RocksDB TransactionDB, and the basic functionality is tested. The next step is to provide support for Optimistic TransactionDB.
Related to: #529 #144