use backend http client #456
Conversation
Generated by 🚫 Danger |
mikz
force-pushed
the
backend-http-client
branch
4 times, most recently
from
2d0cc5e to
0d136f5
Compare
|
|
||
| local http_ng = require('resty.http_ng') | ||
| local user_agent = require('user_agent') | ||
| local resty_url = require('resty.url') | ||
| local resty_env = require('resty.env') | ||
|
|
||
| local _M = { | ||
| endpoint = resty_env.get("BACKEND_ENDPOINT_OVERRIDE") |
There was a problem hiding this comment.
This should be ignoring empty string by using env.value instead.
apicast/src/backend_client.lua
Outdated
|
|
||
| ngx.log(ngx.INFO, 'backend client uri: ', url, ' ok: ', res.ok, ' status: ', res.status, ' body: ', res.body) | ||
|
|
||
| return res | ||
| end | ||
|
|
||
| local authorize_options = { | ||
| headers = { | ||
| ['3scale-options'] = 'rejection_reason_header=1' |
There was a problem hiding this comment.
@davidor this should be more dynamic, right? So we can have no_body for normal authrep but not for oauth calls.
There was a problem hiding this comment.
Let's leave this for a future PR when we actually add more options to avoid adding more complexity to this PR.
| location /transactions/authrep.xml { | ||
| content_by_lua_block { | ||
| ngx.header['3scale-rejection-reason'] = 'limits_exceeded'; | ||
| if ngx.var['http_3scale_options'] == 'rejection_reason_header=1' then |
There was a problem hiding this comment.
Where are we setting ngx.var['http_3scale_options']? I only see ngx.var.options = headers['3scale-options'].
There was a problem hiding this comment.
@davidor $http_ headers are mapped to the request headers: http://nginx.org/en/docs/http/ngx_http_core_module.html#var_http_
davidor
force-pushed
the
backend-http-client
branch
from
a55a92e to
faa8506
Compare
|
@mikz I added a few commits on top of yours. Please let me know if you think that there's anything else we should do before benchmarking this against master. |
|
👍 good job @davidor I think this should be good to go for the performance test. |
|
I benchmarked this. I Deployed just an Apicast (authorizing without going to 3scale's backend) and another one in echo mode (as api backend) in separate machines. The tests were performed using JMeter from an external machine. Absolute numbers are not really meaningful, as there are many factors that can affect them. I just want to focus on the comparison between this branch and its base. I used c4.xlarge aws machines, configured Apicast with 4 workers, and run JMeter for 10 minutes. Without this PR
With this PR
Unfortunately, this PR introduces an important performance regression. |
davidor
force-pushed
the
backend-http-client
branch
3 times, most recently
from
93c73a9 to
d78ddf3
Compare
backend using ngx.location.capture
* remove unnecessary configuration * move authorization to config instead of var
* now there is just 1 location for contacting backend
Authreps now use the 'http_call' location.
- Deletes unused code in the Proxy module. - Avoids defining methods with ':' when self is not necessary in the ngx backend of the http_ng library.
Now http_client is used instead.
gsub is not implemented in the luajit compiler. Replacing gsub with code that can be compiled instead of interpreted will increase performance.
Avoid using regex. It's much more performant this way. Using Apicast in echo mode, in a particular setup it went from ~3.5k rps to ~5k.
davidor
force-pushed
the
backend-http-client
branch
from
d78ddf3 to
4efa022
Compare
|
Update: the last 3 commits increase the performance, but it's still not on par with the performance of the master branch. It's important to note that this benchmark is only testing cpu as Apicast is acting both as auth backend, and api backend. This means that the difference might not be noticeable in a real-case scenario where the api and the auth backend are in different machines. PR: Master: I found that the abstractions in the http_ng/headers module are costly. I did a quick test removing most of the code in that module and got an extra ~1k rps. Maybe we should evaluate whether we need all what that module provides. Also, harcoding the result of the merge operation in the http_ng module gives around ~450 rps more. So that's also something that we could optimize. |
|
👍 |
The members of a 'headers' instance have more methods and make the merge operation more costly. Also, we do not need to instantiate 'headers' here, because it's done later anyway when calling http.request.new(). This also makes the code more readable in my opinion, because we only need to deal with header instances in one class instead of in two like we do now. I have benchmarked this and it increases performance a bit when Apicast acts both as auth backend and api backend (cpu workload).
|
Added changelog entry in 0108fb2 |
TODO