feat(*): major updates to the keyvalue interfaces by Mossaka · Pull Request #30 · WebAssembly/wasi-keyvalue

Mossaka · 2023-11-30T23:45:34Z

This commit has changed many function signatures to account for the scenario where the key does not exist. Instead of returning an error for non-existance keys, the expected behvaior should be returning a none for the client to handle.

The other changes are for providing more clarity to the behavior of the APIs ine wasi:keyvalue interfaces. It explained what it means to have batch operations without guarantees to atomicity and what are atomic operations like CAS.

It also changed the portability criteria to follow the newest WASI proposal template, and added a new goal regarding keyvalue operation performance in this proposal.

It also changed the readwrite and batch interface names to eventual and eventual-batch to indicate the operations are expected to run in an eventual consistent platform.

Closes #24 #25 #9

devigned

Mostly editorial nits. Great work!

keyvalue-handle-watch.md

keyvalue.md

wit/atomic.wit

wit/batch.wit

wit/readwrite.wit

ricochet · 2023-12-04T18:20:42Z

README.md

-Implementations available for at least Windows, Linux & MacOS.
+`wasi:keyvalue` must have at least two complete independent implementations demonstrating
+embeddability in a production key-value store context. The implementations must be able to run
+on at least two different operating systems.


Usually we enumerate linux, macOS, Windows. I'd prefer we don't deviate from that unless necessary

My impression is that, unlike wasi:filesystem and wasi:sockets, the underlying OS doesn't really matter for wasi:keyvalue. Instead, what I think matters quite a lot is that wasi:keyvalue can be implemented in terms of relevant mainstream key-value servers/databases, including both open-source implementations (say Redis or RocksDB or etcd or ?) and proprietary public cloud implementations (say CosmosDB or DynamoDB). Saying just "two" seems to perhaps set the bar too low, although I'm not sure exactly what the right bar is.

Agree that wasi-keyvalue should be able to implement mainstream keyvalue stores. Will add that to the portability criteria.

I've modified the portability criteria to include all three OSes and at least two impl in both open-source and proprietary keyvalue stores. (That means at least 4 impls)

wit/batch.wit

thomastaylor312

Really like this, just some super minor nits I found while reading through

wit/atomic.wit

wit/batch.wit

wit/error.wit

Mossaka · 2023-12-19T23:49:30Z

wit/readwrite.wit

+/// 
+/// A read/write operation is an operation that acts on a single key-value pair.
+/// 
+/// The value in the key-value pair is defined as a `u8` byte array and the intention


This comment is added after all the code reviews to clarify why the value in key-value pair is defined as a list<u8>.

thomastaylor312

These changes all look great to me

devigned

lgtm!

Mossaka · 2024-01-04T18:15:31Z

Could you PTAL? @lukewagner

lukewagner

Great work here! While we probably need more work precisely specifying consistency, this seems like a good starting point for a proposal at this stage. But since names are always harder to change later, I'd like to suggest a few superficial renamings to anticipate future additions; lemme know what you think:

wit/world.wit

wit/readwrite.wit

… get-keys This commit has changed many function signatures to account for the scenario where the key does not exist. Instead of returning an error for non-existance keys, the expected behvaior should be returning a none for the client to handle. The other changes are for providing more clarity to the behavior of the APIs ine `wasi:keyvalue` interfaces. It explained what it means to have batch operations without guarantees to atomicity and what are atomic operations like CAS. It also changed the portability criteria to follow the newest WASI proposal template, and added a new goal regarding keyvalue operation performance in this proposal. Signed-off-by: Jiaxiao Zhou (Mossaka) <duibao55328@gmail.com>

Co-authored-by: David Justice <devigned@users.noreply.github.com>

Signed-off-by: Jiaxiao Zhou (Mossaka) <duibao55328@gmail.com>

thomastaylor312

I'm good with this and think we just need to merge and move forward. One point of dissent I want to record for posterity sake is that I think this interface is the wrong place to be worrying about having different types of consistency guarantees. These are supposed to be for the so-called "80% use case" and I think that just having a basic get/set/del/list along with their batch equivalent should be useful. Elsewhere I've proposed a wasi-cloud-ext world that could contain stricter things around consistency guarantees for a keyvalue interface

Mossaka · 2024-01-18T19:33:32Z

Could you please take another look, @lukewagner?

lukewagner

Thanks for all the consideration! lgtm

Mossaka requested review from devigned, lukewagner and ricochet November 30, 2023 23:45

devigned requested changes Dec 1, 2023

View reviewed changes

ricochet reviewed Dec 4, 2023

View reviewed changes

lukewagner reviewed Dec 5, 2023

View reviewed changes

wit/batch.wit Outdated Show resolved Hide resolved

thomastaylor312 reviewed Dec 19, 2023

View reviewed changes

wit/atomic.wit Outdated Show resolved Hide resolved

wit/batch.wit Outdated Show resolved Hide resolved

wit/error.wit Outdated Show resolved Hide resolved

Mossaka mentioned this pull request Dec 19, 2023

Should we have multiple readwrite interfaces that capture different consistency models? #32

Open

Mossaka commented Dec 19, 2023

View reviewed changes

Mossaka mentioned this pull request Dec 19, 2023

Obtaining the size of a payload should return a result #9

Closed

This was linked to issues Dec 20, 2023

Signature Of KeyValue::Batch::Get-Keys #25

Closed

Obtaining the size of a payload should return a result #9

Closed

Mossaka requested review from devigned, lukewagner, ricochet and thomastaylor312 December 20, 2023 00:01

thomastaylor312 approved these changes Dec 27, 2023

View reviewed changes

devigned approved these changes Jan 3, 2024

View reviewed changes

Mossaka mentioned this pull request Jan 4, 2024

Using resources #33

Merged

lukewagner reviewed Jan 4, 2024

View reviewed changes

wit/world.wit Outdated Show resolved Hide resolved

wit/readwrite.wit Outdated Show resolved Hide resolved

Mossaka and others added 9 commits January 5, 2024 12:57

Apply suggestions from code review

c224a28

Co-authored-by: David Justice <devigned@users.noreply.github.com>

minor change to use Ok(...) throughout the proposals

7f80a9b

Signed-off-by: Jiaxiao Zhou (Mossaka) <duibao55328@gmail.com>

update CI to the newest

124b3d0

Signed-off-by: Jiaxiao Zhou (Mossaka) <duibao55328@gmail.com>

modified the portability criteria

7b7f1e3

Signed-off-by: Jiaxiao Zhou (Mossaka) <duibao55328@gmail.com>

update level of consistency for readwrite and batch interfaces

fab85ad

Signed-off-by: Jiaxiao Zhou (Mossaka) <duibao55328@gmail.com>

change get-keys to keys

464c41c

Signed-off-by: Jiaxiao Zhou (Mossaka) <duibao55328@gmail.com>

regenerate markdowns

bd0769f

Signed-off-by: Jiaxiao Zhou (Mossaka) <duibao55328@gmail.com>

updated README

d77574e

Signed-off-by: Jiaxiao Zhou (Mossaka) <duibao55328@gmail.com>

Mossaka added 2 commits January 5, 2024 13:01

incoming value size returns a result

6c10182

Signed-off-by: Jiaxiao Zhou (Mossaka) <duibao55328@gmail.com>

feat(world): rename keyvalue world to imports

080046e

Signed-off-by: Jiaxiao Zhou (Mossaka) <duibao55328@gmail.com>

Mossaka force-pushed the updates branch from a3cd4fa to 080046e Compare January 5, 2024 21:01

ci: change keyvalue to imports

4b85732

Signed-off-by: Jiaxiao Zhou (Mossaka) <duibao55328@gmail.com>

thomastaylor312 mentioned this pull request Jan 9, 2024

Proposal: Remove distributed-locking (for now) WebAssembly/wasi-cloud-core#9

Closed

chore(*): change readwrite interface name to eventual

d9f4c4c

Signed-off-by: Jiaxiao Zhou (Mossaka) <duibao55328@gmail.com>

Mossaka requested review from devigned, lukewagner and thomastaylor312 January 16, 2024 21:11

Mossaka changed the title ~~feat(*): updated comments and changed signatures of get, get-many and get-keys~~ feat(*): major updates to the keyvalue interfaces Jan 16, 2024

chore(*): update the markdown files

2cedc65

Signed-off-by: Jiaxiao Zhou (Mossaka) <duibao55328@gmail.com>

thomastaylor312 approved these changes Jan 18, 2024

View reviewed changes

lukewagner approved these changes Jan 19, 2024

View reviewed changes

Mossaka merged commit 9cde2eb into main Jan 19, 2024

Mossaka deleted the updates branch January 19, 2024 22:01

Conversation

Mossaka commented Nov 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

devigned left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ricochet Dec 4, 2023

Choose a reason for hiding this comment

Uh oh!

lukewagner Dec 5, 2023

Choose a reason for hiding this comment

Uh oh!

Mossaka Dec 6, 2023

Choose a reason for hiding this comment

Uh oh!

Mossaka Dec 19, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

thomastaylor312 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Mossaka Dec 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thomastaylor312 left a comment

Choose a reason for hiding this comment

Uh oh!

devigned left a comment

Choose a reason for hiding this comment

Uh oh!

Mossaka commented Jan 4, 2024

Uh oh!

lukewagner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

thomastaylor312 left a comment

Choose a reason for hiding this comment

Uh oh!

Mossaka commented Jan 18, 2024

Uh oh!

lukewagner left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Mossaka commented Nov 30, 2023 •

edited

Loading

Mossaka Dec 19, 2023 •

edited

Loading