Implement wasi-keyvalue

[iawia002]

The wasi-keyvalue API has entered Phase 2. This patch implements the API, giving wasm components access to key-value storages.

Currently, only the Redis storage backend has been implemented, and support for more kv storage (e.g. etcd, etc.) will be added in the future.

cc @alexcrichton

[bjorn3]

Would it make sense to have a local storage method too? That would allow quick tests without requiring the setup of an entire database.

[iawia002]

Would it make sense to have a local storage method too? That would allow quick tests without requiring the setup of an entire database.

Sure, I'll implement an in-memory backend and make the other database backends optional.

[iawia002]

I will fix the dead-code warning, but I have no idea why the runtime_config_get test failed:

---- runtime_config_get stdout ----
thread 'runtime_config_get' panicked at crates/wasmtime/src/runtime/vm/libcalls.rs:147:5:
internal error: entered unreachable code

[iawia002]

I will fix the dead-code warning, but I have no idea why the runtime_config_get test failed:

---- runtime_config_get stdout ----
thread 'runtime_config_get' panicked at crates/wasmtime/src/runtime/vm/libcalls.rs:147:5:
internal error: entered unreachable code

I have figured it out (#8997) and made a temporary solution by adding the wasmtime/wmemcheck feature to wasi-runtime-config:

[features]
# Do not use, this is only used for testing in CI,
# see https://github.com/bytecodealliance/wasmtime/issues/8997
test = ["wasmtime/wmemcheck"]

[alexcrichton]

This is looking quite good to me, thanks for the PR and the work here! The biggest question below I have is about how buckets are opened.

[iawia002]

All comments addressed ec3373a, PTAL

[iawia002]

All comments addressed b9d2bd2, PTAL.

The CI failed due to the feature combinations issue we encountered before. Since the changes are not related to this PR, I have submitted a separate PR #9016 to address that issue. Once that PR is merged, I will rebase this PR, and the CI should be happy then.

[alexcrichton]

Looks good to me, thanks! I'll queue for merge after a rebase makes CI happy

(thanks for helping to debug CI and fix the issue!)

[iawia002]

Hi @alexcrichton, CI is happy now except cargo vet, I believe that should be handled by you.

[thomastaylor312]

Hey there! wasi-keyvalue champion here. So I don't think any of the champions knew this was landing. I'm super excited someone implemented it here (thanks @iawia002)! However, we are in the process of incorporating a last round of feedback now that we've had people implementing against it: WebAssembly/wasi-keyvalue#46. We might want to hold off on releasing this until we can make those changes and go to a full 0.2.0 release of wasi-keyvalue.

Also, an in memory KV store makes sense to me, but I don't think we want to take hard deps on other KV stores in something like wasmtime. This was something that we had loosely talked about as being "host plugins" using something like wRPC

[iawia002]

Hi @thomastaylor312, basically, when a proposal enters Phase 2, we can start implementing it. If we wait for it to be completely ready before we begin, the timeline would become excessively long. Therefore, we also have corresponding implementations for draft versions of proposals, allowing users to start experimenting with the API.

For the wasi-keyvalue, the implementation targets a specific tag 219ea36. I will continue to track the latest changes in this proposal to keep the implementation up to date.

wasmtime/ci/vendor-wit.sh

Line 61 in ba864e9

make_vendor "wasi-keyvalue" "keyvalue@219ea36"

Regarding the release issue, strictly speaking, this has not yet been landed. According to the wasmtime release schedule, the code will be frozen on the 5th and will not be released until the 20th of this month. So we still have time to follow up on the latest changes in the wasi-keyvalue proposal.

Overall, I think we can go ahead and release this, allowing users of wasmtime v24 to try out the wasi-keyvalue@0.2.0-draft API.

but I don't think we want to take hard deps on other KV stores in something like wasmtime. This was something that we had loosely talked about as being "host plugins" using something like wRPC

I agree with this, there are too many KV storage solutions. But I'm not sure if we should move all KV storage implementations out of wasmtime, because depending on Redis/etcd/Memcached, etc., does not affect wasmtime itself. It only affects the standalone crate wasmtime-wasi-keyvalue and the wasmtime-cli. I hope to implement some very popular providers in wasmtime and turn the others into "host plugins".

[thomastaylor312]

Yeah we chatted about this today in the wasmtime meeting. So I think we are ok with the current pinned tag, especially since things are behind a feature flag.

However, I still think keeping Redis in tree is a bad idea. The reason why even the super popular ones should be a plugin type model is that now any time the redis client has a bug, you have to update the keyvalue crate and the CLI just to get the bug fix. I know this could happen with any crate, but with client APIs, these kinds of fixes happen quite often. If it happens to be security sensitive, it would necessitate a release of the crate and CLI for anyone depending on it. This wouldn't happen if this was done by loading a plugin separately

[iawia002]

I have now seen the meeting notes. I apologize for any inconvenience caused by not notifying you during the implementation. I will submit a PR to revert the introduction of Redis.

[thomastaylor312]

No problem! I just wanted to make sure we had our ducks in a row here as we move forward adding new interfaces