-
Notifications
You must be signed in to change notification settings - Fork 11
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
feat(*)!: Adds cas operation and fixes keys cursor #46
base: main
Are you sure you want to change the base?
feat(*)!: Adds cas operation and fixes keys cursor #46
Conversation
Marked this as a draft until we've finished any conversation in #44. Also cc @kate-goldenring for visibility |
This PR incorporates some feedback received on the current draft interface. First off, this changes the cursor for listing keys to be a string instead of a number in order to support more systems. The second changes adds back in a CAS operation as multiple people had been asking for it. See the discussion in WebAssembly#44 for more context. Because these are breaking changes and there are multiple draft implementations out there, I bumped this to draft2. Ideally we should be able to move to an RC soon with these changes added in Closes WebAssembly#44 Closes WebAssembly#45 Signed-off-by: Taylor Thomas <taylor@cosmonic.com>
e24291a
to
9915d37
Compare
I did a little noodling with @devigned and came up with something much cleaner. Curious what people think |
When checking this operation across stores, I noticed that everything was using signed numbers so you can increment or decrement as needed Signed-off-by: Taylor Thomas <taylor@cosmonic.com>
Also, one last change after looking at the increment functionality across databases. The number and the delta should be signed integers so the counter can be decremented as needed |
With Spin, right now, we are only focused on implementing the |
resource cas { | ||
/// Construct a new CAS operation. Implementors can map the underlying functionality | ||
/// (transactions, versions, etc) as desired. | ||
new: static func(bucket: borrow<bucket>, key: string) -> result<cas, error>; | ||
/// Get the current value of the key (if it exists). This allows for avoiding reads if all | ||
/// that is needed to ensure the atomicity of the operation | ||
current: func() -> result<option<list<u8>>, error>; | ||
} |
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 am curious to hear your motivations / rational behind the cas resource design? My feeling for that when I first saw it was complicated and more unintuitive than a simple operation like compare-and-swap: func(key: string, old: option<list<u8>>, new: option<list<u8>>) -> result<bool, error>;
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.
This gives a lot more flexibility for how implementors can solve this. As seen in the survey of keyvalue stores I did, the most common things were transactions or revision_id-based options, along with some direct comparing and swapping. Representing this as a resource allows implementations to keep track of state for the operation. So for each of the 3 aforementioned types of operation, this is what is would roughly look like:
- Transactions: An implementation can start a transaction and link it to a resource. You can still fetch the current value if you wish to do a comparison, but otherwise can use the guarantee of the transaction to ensure that you aren't clobbering a write
- Revision IDs: Implementations can fetch the current revision of the key along with the value (or lazily load the value if
current
is called) and keep that revision ID around as its state. Then when the value is swapped it can use the revision ID - Direct compare and swap: The implementation wouldn't actually have any state but could fetch the value and do the comparison on the back end. As mentioned, this was by far the least used option among all the stores I surveyed
Forcing everyone to do a direct compare and swap means they have to a) fetch the value and b) send the other value. This could be much less efficient, especially if the values are large. It basically forces everything to implement it using a much less efficient way (that is much more vulnerable to the ABA problem you mentioned below) even though it is the least common option. The other options at least reduce the possibility of or eliminate the ABA problem as well, addressing your other comment)
|
||
/// Perform the swap on a CAS operation. This consumes the CAS handle and returns an error if | ||
/// the CAS operation failed. | ||
swap: func(cas: cas, value: list<u8>) -> result<_, cas-error>; |
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.
A common problem for the implementation of compare-and-swap
operation is that it suffers from the ABA problem. I am hoping to see documentation of this API to guide the host implementors on the expectation. Just mentioning "this API might suffer from the ABA problem" is fine to me.
This PR incorporates some feedback received on the current draft interface. First off, this changes the cursor for listing keys to be a string instead of a number in order to support more systems.
The second changes adds back in a CAS operation as multiple people had been asking for it. See the discussion in #44 for more context.
Because these are breaking changes and there are multiple draft implementations out there, I bumped this to draft2. Ideally we should be able to move to an RC soon with these changes added in
Closes #44
Closes #45