-
Notifications
You must be signed in to change notification settings - Fork 14
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
kv: iterator support #629
base: develop
Are you sure you want to change the base?
kv: iterator support #629
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.
Seems reasonable to me. I'd recommend closing the iterator once you hit done = true
rather than requiring the consumer to call IterClose
.
In general DashMap
s make me nervous cuz we've had a lot of deadlocks from them, but just looking at the code it seems ok 😅
Please be sure to hit all the new codepaths here in tests, ideally with two processes to try to exercise the DashMap
s in a multi-tokio::task context.
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.
Comment nits, please try to give interface that final polish.
Echoing Nick's concern here about lockups when multiple processes are iterating on same DB, please make sure to test that.
lib/src/kv.rs
Outdated
@@ -20,6 +20,10 @@ pub enum KvAction { | |||
BeginTx, | |||
Commit { tx_id: u64 }, | |||
Backup, | |||
// Iterator operations |
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.
Comments inside enums should be ///
docstrings -- please label each iter operation with an explanation of how to use it. It's not immediately obvious to me how it works.
Problem
hyperware-ai/process_lib#125
Solution
Testing