datasource: use async data source for wasm#216
Conversation
5de0363 to
a5cc183
Compare
9a8b1bc to
66b784b
Compare
320163f to
f8ab13b
Compare
Signed-off-by: crazyxy <yxyan@google.com>
|
@jplevyak @PiotrSikora the PR is ready for review. PTAL. Thanks! |
|
@yxue could you merge |
Signed-off-by: crazyxy <yxyan@google.com>
Signed-off-by: Yan Xue <yxyan@google.com>
| auto tls_slot = context.threadLocal().allocateSlot(); | ||
| // Create a base WASM to verify that the code loads before setting/cloning the for the | ||
| // individual threads. | ||
| auto plugin = std::make_shared<Common::Wasm::Plugin>( |
There was a problem hiding this comment.
Can we pull this out of the callback ?
There was a problem hiding this comment.
If code is not included in the Plugin, we can. I added code in the Plugin in this PR so we cannot. But we can keep code out of the Plugin. Which one would you prefer?
There was a problem hiding this comment.
Can you pass it into Common::Wasm::createWasm(). The plugin is supposed to have only information specific to the particular plugin point, while the code is only used to create the initial Wasm (or non-cloneable thread-local Wasm(s)) but in any case is common to all plugin points. The new PR I submitted uses the copy constructor of Wasm for all the thread-local versions of the VMs, so you can store the code in Wasm and then access it if necessary in the copy constructor, but do not store it in the copy constructed Wasm(s) because it could be big and we don't want to waste the memory. The code in the base_wasm will be deleted when the last thread-local Wasm is created.
There was a problem hiding this comment.
@jplevyak update the PR to pass the code to Common::Wasm::createWasm() instead of putting it in Plugin
Signed-off-by: Yan Xue <yxyan@google.com>
| Common::Wasm::getOrCreateThreadLocalWasm(*base_wasm, *configuration, dispatcher)); | ||
| }); | ||
| return std::make_shared<WasmAccessLog>(root_id, std::move(tls_slot), std::move(filter)); | ||
|
|
There was a problem hiding this comment.
Can you encapsulate or create a function (e.g. in wasm.h/cc) to do this repeated code? We have a number of plugin points and are going to have more so it will be repeated several more times.
Signed-off-by: Yan Xue <yxyan@google.com>
|
As discussed offline we need a way of propagating the (currently) exception errors that are now uncaught in the async operation callback back to xDS probably via the init manager. Prior to this change they would have occurred inline during configuration and be caught. Now they will causes Envoy to exit. |
|
OK, LGTM, but I would like to get Piotr's opinion. @PiotrSikora |
PiotrSikora
left a comment
There was a problem hiding this comment.
@yxue could you merge with master?
Signed-off-by: crazyxy <yxyan@google.com>
Signed-off-by: crazyxy <yxyan@google.com>
|
@jplevyak can you resolve your "change requested" and merge this? Thanks! |
Signed-off-by: Lizan Zhou <lizan@tetrate.io>
* Update envoy-wasm sha Signed-off-by: Lizan Zhou <lizan@tetrate.io> * fix due to ABI change * remove compile db Signed-off-by: Lizan Zhou <lizan@tetrate.io> * fix for envoyproxy/envoy-wasm#216 Signed-off-by: Lizan Zhou <lizan@tetrate.io> * fix ci * roll forward Signed-off-by: Lizan Zhou <lizan@tetrate.io>
Use async data source
Description: use async data source
Risk Level: Med
Testing: Unit test
Docs Changes:
Release Notes:
Fixes #Issue: Fix #187
[Optional Deprecated:]