-
Notifications
You must be signed in to change notification settings - Fork 4
initial interface definition for distributed locks #3
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: David Justice <david@devigned.com>
| @@ -1,10 +1,10 @@ | |||
| # `wasi-distributed-lock-service` | |||
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.
Love this!
| ### Phase 4 Advancement Criteria | ||
|
|
||
| `wasi-distributed-lock-service` should have at least two implementations (i.e., from service providers, and or cloud providers), and, at the very minimum, pass the testsuite for Windows, Linux, and MacOS. | ||
| `wasi-distributed-lock` should have at least two implementations (i.e., from service providers, and or cloud providers), and, at the very minimum, pass the test suite for Windows, Linux, and MacOS. |
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.
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.
etcd and consul are probably worthwhile impls too (both have their issues by nature of their Raft implementations, but are good enough for many).
|
|
||
| /// acquire will attempt to obtain a lease for a given string key for a time to live (ttl) | ||
| /// in milliseconds. | ||
| acquire: func(key: string, ttl: milliseconds) -> result<lock, 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.
What's your thoughts on acquiring a distributed lock and getting a lock and a fencing token back?
fencing token is a way to ensure a write to the persistent service that the lock may be dealyed. See "Making the lock safe with fencing" in How to Do Disitrbuted Locking
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.
+1 on encouraging fencing tokens - I'm not sure everyone has support for them (also for cases like redis, where gettimeofday is used for expiration tracking, rather than a monotonic clock) so I'm not sure how we could generically expose them :(
|
I would like to mention a paper from Google that descirbes Chubby service is very relevent to the proposal here. It helps me to understand the rational for a lock service. |
Great paper. I definitely had this open prior to authoring the interface. |
| ### Introduction | ||
|
|
||
| The `wasi-distributed-lock-service` world aim to provide a set of generic interfaces for distributed lock services which provide mechanisms to ensure that only one process can mutate a particular state at a time. They are often used to prevent race conditions in scenarios where two or more processes attempting to update the same state at the same time that could result in data inconsistencies. | ||
| The `wasi-distributed-lock` world aim to provide a set of generic interfaces for distributed locking which provides mechanisms to ensure that only one process can mutate a particular state at a time. They are often used to prevent race conditions in scenarios where two or more processes attempting to update the same state at the same time that could result in data inconsistencies. |
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.
| The `wasi-distributed-lock` world aim to provide a set of generic interfaces for distributed locking which provides mechanisms to ensure that only one process can mutate a particular state at a time. They are often used to prevent race conditions in scenarios where two or more processes attempting to update the same state at the same time that could result in data inconsistencies. | |
| The `wasi-distributed-lock` world aims to provide a set of generic interfaces for distributed locking which provides mechanisms to ensure that only one process can mutate a particular state at a time. They are often used to prevent race conditions in scenarios where two or more processes attempting to update the same state at the same time that could result in data inconsistencies. |
~nit
| /// Errors that can be returned from the locker | ||
| variant error { | ||
| /// not-acquired error indicates the lock was not acquired because it has already been | ||
| /// acquired by another entity. | ||
| not-acquired(string), | ||
| /// already-released error indicates the lease for the lock has already expired. | ||
| lease-expired(string), | ||
| /// already-released error indicates the lease for the loack has already been released. | ||
| already-released(string), | ||
| /// unknown indicates an unexpected error occurred when acquiring or releasing a lock | ||
| unknown(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.
Rather than variants for errors, consider using wasi-cloud error. Like, here:
https://github.com/WebAssembly/wasi-keyvalue/blob/0bce169f659170379aa4aeceef0e83711671acb9/wit/types.wit#L30
wasi-cloud-error looks like so:
https://github.com/WebAssembly/wasi-keyvalue/blob/0bce169f659170379aa4aeceef0e83711671acb9/wit/deps/io/error.wit#L1-L34
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 actually think in this case a variant is much more descriptive and useful for a consumer. I don't mind if the inner error is a wasi-cloud-error resource instead of a string, but I do like keeping variants around
| **TODO** | ||
|
|
||
| ### References and Acknowledgements | ||
| **TODO** |
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.
~consider adding example usages of this interface, like:
https://github.com/WebAssembly/wasi-messaging/blob/main/examples.md
thomastaylor312
left a comment
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.
Overall I like how simple and straightforward this is. Just calling out here that per discussion going on in wasi-cloud-core, we're thinking of maybe putting this in a wasi-cloud-ext
Introduce the first take on the WASI distributed lock interface.
Todos possibly to be targeted in a follow up PR.