Skip to content

Add support for async requests for wasm plugin#9896

Merged
shukitchan merged 10 commits intoapache:masterfrom
shukitchan:wasmasync
Jul 10, 2023
Merged

Add support for async requests for wasm plugin#9896
shukitchan merged 10 commits intoapache:masterfrom
shukitchan:wasmasync

Conversation

@shukitchan
Copy link
Contributor

No description provided.

@shukitchan shukitchan added this to the 10.0.0 milestone Jun 21, 2023
@shukitchan shukitchan self-assigned this Jun 21, 2023
@shukitchan
Copy link
Contributor Author

[approve ci autest]

@shukitchan shukitchan marked this pull request as ready for review June 26, 2023 05:44
@bryancall bryancall requested a review from serrislew June 26, 2023 22:33
@shukitchan
Copy link
Contributor Author

There are quite a lot of materials on async requests in a wasm module in this talk - https://events.istio.io/istiocon-2021/slides/c8p-ExtendingEnvoyWasm-EdSnible.pdf

TSDebug(WASM_DEBUG_TAG, "[%s] no context or not yet reenabled transaction", __FUNCTION__);

if (result == 0) {
TSHttpTxnReenable(txnp, TS_EVENT_HTTP_CONTINUE);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does context->reenable_txn_ not need to get set here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I should do that. Will make a change.


* Getting and setting trailer request and response header
* Getting and setting HTTP/2 frame meta data
* Support asynchronous request call in the start handler function of the transaction lifecycle
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be in limitations? I thought this is a new feature but adding it here implies it is not supported

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. This is a new feature but there is no easy way for us to support async request inside the "start" handler function of the plugin lifecycle.

Let me reword this a bit.

@shukitchan shukitchan requested a review from serrislew June 28, 2023 17:18
@shukitchan
Copy link
Contributor Author

It is ready for review again. thanks.

@bneradt
Copy link
Contributor

bneradt commented Jun 29, 2023

[approve ci clang-format]

@shukitchan
Copy link
Contributor Author

[approve ci autest]

serrislew
serrislew previously approved these changes Jul 5, 2023
@shukitchan shukitchan merged commit a2e8615 into apache:master Jul 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants