Skip to content
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

option to drop post_action #290

Merged
merged 11 commits into from
Oct 17, 2017
Merged

option to drop post_action #290

merged 11 commits into from
Oct 17, 2017

Conversation

mikz
Copy link
Contributor

@mikz mikz commented Mar 9, 2017

#289 revealed limitations of post_action

this PR offers new experimental flag to turn off post_action
and use nginx timers instead

setting APICAST_REPORTING_THREADS to a value greater than 0 is going to enable asynchronous reporting to backend.


doing synchronous call in the post_action will
push the latency to the client on the same connection

using timers we can make that fully asynchronous
not being tied to the request phase at all

setting APICAST_REPORTING_THREADS environment variable
to a number greater than 0 will enable reporting in asynchronous
timers instead of synchronous call

the number defines a maximum number of parallel calls
to backend per worker

if there is new report coming but no connections is available
APIcast will wait for up to 10 seconds before falling back to the
synchronous call

please note this is experimental feature

TODO

@octobot
Copy link

octobot commented Mar 9, 2017

Spell Checker found issues

doc/management-api.md

Line Typo
32 See [example-config.json](../examples/configurati
32 See [example-config.json](../examples/configuration/ex

doc/parameters.md

Line Typo
3 APIcast v2 has a number of parameters co
3 ariables) that can modify the behavior of the gateway. The following
5 e that when deploying APIcast v2 with OpenShift, some of thee
15 Default: **stderr**
17 or more information. The file pathcan be either absolute, or relati
21 nfo
21 warn
46 Default: "apicast"
77 Values: a number > **60**
78 Default: 0
80 r. The value should be set to 0 or more than 60. For example,
80 ould be set to 0 or more than 60. For example, if `APICAST_CON
80 ONFIGURATION_CACHE` is set to 120, the gateway will reload the
80 eload the configuration every 2 minutes (120 seconds).
80 onfiguration every 2 minutes (120 seconds).
84 Default: "127.0.0.1"
86 ning Redis instance for OAuth 2.0 flow. REDIS_HOST parameter
90 Default: 6379
92 ning Redis instance for OAuth 2.0 flow. REDIS_PORT parameter
98 ning Redis instance for OAuth 2.0 flow. REDIS_URL parameter c
98 e used to set the full URI as DSN format like: `redis://PASSWOR
103 Default: 604800
105 uthenticate using OAuth, this param specifies the TTL (in seconds
109 pty, the DNS resolver will be autodiscovered.
161 Setting it to particual version will make it not auto
173 tificate bundle generated by [export-builtin-trusted-certs](https://github.com/openresty
177 Default: 0
178 Value:: integer >= 0
180 Value greater than 0 is going to enable out-of-ban
183 ronous reports can be running simultainesly

Spell Checker found issues

doc/management-api.md

Line Typo
32 See [example-config.json](../examples/configurati
32 See [example-config.json](../examples/configuration/ex

doc/parameters.md

Line Typo
3 APIcast v2 has a number of parameters co
3 ariables) that can modify the behavior of the gateway. The following
5 e that when deploying APIcast v2 with OpenShift, some of thee
15 Default: **stderr**
17 or more information. The file pathcan be either absolute, or relati
21 nfo
21 warn
46 Default: "apicast"
77 Values: a number > **60**
78 Default: 0
80 r. The value should be set to 0 or more than 60. For example,
80 ould be set to 0 or more than 60. For example, if `APICAST_CON
80 ONFIGURATION_CACHE` is set to 120, the gateway will reload the
80 eload the configuration every 2 minutes (120 seconds).
80 onfiguration every 2 minutes (120 seconds).
84 Default: "127.0.0.1"
86 ning Redis instance for OAuth 2.0 flow. REDIS_HOST parameter
90 Default: 6379
92 ning Redis instance for OAuth 2.0 flow. REDIS_PORT parameter
98 ning Redis instance for OAuth 2.0 flow. REDIS_URL parameter c
98 e used to set the full URI as DSN format like: `redis://PASSWOR
103 Default: 604800
105 uthenticate using OAuth, this param specifies the TTL (in seconds
109 pty, the DNS resolver will be autodiscovered.
161 Setting it to particual version will make it not auto
173 tificate bundle generated by [export-builtin-trusted-certs](https://github.com/openresty
177 Default: 0
178 Value:: integer >= 0
180 Value greater than 0 is going to enable out-of-ban
183 ronous reports can be running simultainesly

Generated by 🚫 Danger

@@ -113,13 +113,28 @@ function _M.disabled()
ngx.exit(ngx.HTTP_FORBIDDEN)
end

function _M.info()

Choose a reason for hiding this comment

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

This looks separate from no post_action changes, adding an /info endpoint, no?

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 it is separate commit. Not really useful without the post_action changes because there where no timers before.

@mikz mikz force-pushed the semaphores branch 8 times, most recently from 1c56134 to dd8276e Compare March 20, 2017 13:47
@mikz mikz removed this from the On-premise ER5 release milestone Mar 20, 2017
[nginx] increase number of running timers

[nginx] increase default pool size for sockets
that returns information about timers
doing synchronous call in the post_action will
push the latency to the client on the same connection

using timers we can make that fully asynchronous
not being tied to the request phase at all

setting APICAST_REPORTING_THREADS environment variable
to a number greater than 0 will enable reporting in asynchronous
timers instead of synchronous call

the number defines a maximum number of parallel calls
to backend per worker

if there is new report coming but no connections is available
APIcast will wait for up to 10 seconds before falling back to the
synchronous call

please note this is **experimental** feature
so SSL validation will work

$ openssl ecparam -genkey -name prime256v1 -out key.pem
$ openssl req -new -sha256 -key key.pem -out csr.csr -subj '/CN=127.0.0.1'
$ openssl req -x509 -sha256 -days 3650 -key key.pem -in csr.csr -out certificate.pem
@davidor
Copy link
Contributor

davidor commented Oct 17, 2017

I ran some tests to make sure that setting APICAST_REPORTING_THREADS=0 does not introduce a performance regression in the normal Apicast flow.

I performed 2 tests, each of them comparing Apicast master branch with this PR:

  1. Deployed an Apicast, an Apicast in echo mode (as api backend), and a 3scale backend. All of them in different machines. In this scenario Apicast is limited by the 3scale backend.
  2. Deployed just an Apicast and another one in echo mode (as api backend) in separate machines.

The tests were performed using JMeter from an external machine. There are many factors that can affect the throughput, latencies, etc. So absolute numbers are not really meaningful. I just want to focus on the comparison between the master branch and this PR. I used c4.xlarge aws machines, configured Apicast with 4 workers, and run JMeter for 5 minutes.

Results for test 1:
Apicast master
1_master

PR
1_pr

Results for test 2:
Apicast master
2_master

PR
2_pr

The results are similar in both cases.

@andrewdavidmackenzie
Copy link
Member

andrewdavidmackenzie commented Oct 17, 2017

So, focussing on the second of the two comparisons:

In the Pr version:

Thruput is 97.7% of Master
95%ile latency is 107% of Master
99%ile latency is 92% of Master
Max latency is 177% of Master

If it solves a real problem for this and other customers with long latencies, then I guess 97.7% is acceptable....and maybe their are opportunities to optimize it further....

@mikz mikz merged commit 4661740 into master Oct 17, 2017
@mikz mikz deleted the semaphores branch October 17, 2017 11:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants