-
Notifications
You must be signed in to change notification settings - Fork 101
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
Add Promise support for http callout #265
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: jizhuozhi.george <[email protected]>
fix ci Signed-off-by: jizhuozhi.george <[email protected]>
ci add `http_parallel_call` Signed-off-by: jizhuozhi.george <[email protected]>
|
fix ci Signed-off-by: jizhuozhi.george <[email protected]>
Thanks! I'll fix existing errors shortly. Sorry about that! |
move promise and dispatch http request to callout folder Signed-off-by: jizhuozhi.george <[email protected]>
remove Chinese comments Signed-off-by: jizhuozhi.george <[email protected]>
There is currently a problem involving the collaboration between SDK and user code: I have no idea to resolve this problem :( |
simplify http_parallel_call example Signed-off-by: jizhuozhi.george <[email protected]>
fix ci Signed-off-by: jizhuozhi.george <[email protected]>
fix example cloning self to move but not using hostcalls Signed-off-by: jizhuozhi.george <[email protected]>
fix licenses and bazel (maybe) Signed-off-by: jizhuozhi.george <[email protected]>
Bazel BUILD file has been reformatted :) |
Signed-off-by: jizhuozhi.george <[email protected]>
I think (but didn't check the code) that this boilerplate could be included in the dispatcher, and executed automatically when events for registered callouts are received from the host... or am I missing something? |
@@ -0,0 +1,341 @@ | |||
// Copyright 2024 Google LLC |
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.
Did you look at std::future
and/or futures-rs
crate? It seems like they are implementing the same concepts, so I'm wondering how much of the Promise code we could avoid?
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.
std::future
in c++ is an async primitive to get result in multi-thread programming. std::future
produced by std::async
or std::promise
. Once program calls std::future::get
, it will block until std::async
completed or std::promise::set
called. It means at least two threads for provider and consumer but wasm has no (once call dispatched, thread will be used to process other requests).
And future-rs
is providing the foundations for asynchronous programming in Rust, and lazy executed which requires an executor to poll the result (which means all operations must be non-blocking). The problem as same asstd::future
, there is no thread as executor to poll the result.
So the only way that we could own the thread is as callback of http response.
The Go SDK handles it this way, but I'm not sure if doing so breaks the Rust SDK API |
In the current envoy WASM plugins, if we need IO request such as HTTP/GRPC/Redis, we must register request to envoy event loop and assigned a token via such as
dispatch_http_call
and WASM plugin should yield current request lifetime usingAction::Pause
. When the IO request completed, the envoy will callback to WASM plugin viaon_http_call_response
, and plugin should dispatch response using token (or ignored if single IO request). If we want to share something betweendispatch_http_call
andon_http_call_response
, we must share them in plugin context fields, it's not a suitable scope.Here is an example from proxy-wasm-rust-sdk
So there is three major problem we need to resolve:
In Rust async programming, normally we use
async/await
for IO request, but in envoy WASM plugin, there is no executor to poll future. A suitable solution is providing JavaScript style Promise (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise). With Promise, we could write all logic in single functionIt seems more fluid then writing in callback of
on_http_call_response
, but there is no executor for promise to trigger state transferring. We can useon_http_call_response
as trigger simplyAs for making relationship between multi tokens and promises, we could just using
HashMap
withinsert/remove
(maybe it should be embed in SDK but not belongs to this PR)Note:
hostcalls
is re-implemented ofresume_http_request
andsend_http_response
because we cannot moveself
to callbacks.