feature: add the balancer.recreate_request function, which allows user to recreate request buffer in balancer phase.
#302
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@dndx just a reminder, the CI failed. |
dndx
force-pushed
the
proxy_recreate_request
branch
from
ef551a4 to
20204f1
Compare
|
@doujiang24 Yes the test plan was wrong. I have fixed the number. |
dndx
force-pushed
the
proxy_recreate_request
branch
from
20204f1 to
5a27cf2
Compare
dndx
added a commit
to Kong/kong-build-tools
that referenced
this pull request
Jun 30, 2020
…ocao These will be removed once openresty/lua-nginx-module#1734 and openresty/lua-resty-core#302 are merged
dndx
added a commit
to Kong/kong-build-tools
that referenced
this pull request
Jul 1, 2020
…ocao These will be removed once openresty/lua-nginx-module#1734 and openresty/lua-resty-core#302 are merged
|
Ping @doujiang24. Could you give another look at this? Thanks! |
thibaultcha
requested changes
Jul 16, 2020
lib/ngx/balancer.lua
Outdated
| @@ -38,6 +38,10 @@ if subsystem == 'http' then | |||
| int ngx_http_lua_ffi_balancer_set_timeouts(ngx_http_request_t *r, | |||
| long connect_timeout, long send_timeout, | |||
| long read_timeout, char **err); | |||
|
|
|||
| int | |||
| ngx_http_lua_ffi_balancer_recreate_request(ngx_http_request_t *r, | |||
There was a problem hiding this comment.
style: FFI prototype declarations should have the return type on the same line, like in the C module
lib/ngx/balancer.md
Outdated
| @@ -237,6 +237,29 @@ This function was first added in the `0.1.7` version of this library. | |||
|
|
|||
| [Back to TOC](#table-of-contents) | |||
|
|
|||
| recreate_request | |||
| ------------ | |||
There was a problem hiding this comment.
style: this line should underline the whole method name
lib/ngx/balancer.md
Outdated
|
|
||
| **context:** *balancer_by_lua** | ||
|
|
||
| Regenerates the request buffer for sending to the upstream server. This is useful, for example |
There was a problem hiding this comment.
Maybe better to stay consistent with wording? i.e. use "recreate" instead of "regenerate" which is the terminology used everywhere else when referring to this API.
lib/ngx/balancer.md
Outdated
|
|
||
| Normally this does not work because the request buffer is created once during upstream module | ||
| initialization and won't be regenerated for subsequent retries. However you can use | ||
| `proxy_set_header My-Header $my_header` and sets the `ngx.var.my_header` variable inside the |
lib/ngx/balancer.md
Outdated
| Normally this does not work because the request buffer is created once during upstream module | ||
| initialization and won't be regenerated for subsequent retries. However you can use | ||
| `proxy_set_header My-Header $my_header` and sets the `ngx.var.my_header` variable inside the | ||
| balancer phase. Calling this function after that will cause the request buffer to be re-generated |
There was a problem hiding this comment.
"Calling this function after that" can be confusing for the reader. "Calling balancer.recreate_request() after updating a header field will cause..."
lib/ngx/balancer.md
Outdated
| and the `My-Header` header will thus contain the new value. | ||
|
|
||
| **Warning:** because the request buffer has to be recreated and such allocation occurs on the | ||
| request memory pool, the old buffer has to be thrown away and only be freed after the request |
There was a problem hiding this comment.
and will only be freed when...
to recreate request buffer in balancer phase. This allows certain request parameters, such as headers (including `Host` header) to be modified between balancer retries.
dndx
force-pushed
the
proxy_recreate_request
branch
from
5a27cf2 to
1e749c8
Compare
|
Thanks for the reviews @thibaultcha, I fixed the styling and ran the document through with |
doujiang24
approved these changes
Jul 18, 2020
syzh
approved these changes
Jul 19, 2020
agentzh
approved these changes
Jul 20, 2020
agentzh
requested changes
Jul 20, 2020
lib/ngx/balancer.md
Outdated
| to this function should be made **only** if you know the request buffer must be regenerated, | ||
| instead of unconditionally in each balancer retries. | ||
|
|
||
| This function was first added in the `0.1.19` version of this library. |
There was a problem hiding this comment.
This should be changed to 0.1.20.
agentzh
requested changes
Jul 20, 2020
t/balancer.t
Outdated
| print("here") | ||
| local b = require "ngx.balancer" | ||
|
|
||
| if ngx.ctx.balancer_ran then |
There was a problem hiding this comment.
I think balancer_run is more grammatically correct.
doujiang24
pushed a commit
that referenced
this pull request
Jul 30, 2020
…ser to recreate request buffer in balancer phase. (#302) This allows certain request parameters, such as headers (including `Host` header) to be modified between balancer retries.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I hereby granted the copyright of the changes in this pull request
to the authors of this lua-resty-core project.
Feature was originally developed by @locao inside Kong/lua-kong-nginx-module#13. With slight modifications from @dndx.
Related PR: openresty/lua-nginx-module#1734.