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

[policy] trigger post_action phase #539

Merged
merged 6 commits into from
Jan 12, 2018
Merged

[policy] trigger post_action phase #539

merged 6 commits into from
Jan 12, 2018

Conversation

mikz
Copy link
Contributor

@mikz mikz commented Jan 4, 2018

By sharing ngx.ctx with the subrequest we are able to
share the policy chain and execute the post_action phase.

It should be safe, but we should focus on trying this during QE process.

TODO

Performance

Before

wrk --connections 10 --threads 10 --duration 60 'http://localhost:8080/?user_key=foo'
Running 1m test @ http://localhost:8080/?user_key=foo
  10 threads and 10 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency     5.25ms    0.93ms  17.38ms   69.83%
    Req/Sec   191.24     16.50   393.00     73.23%
  114377 requests in 1.00m, 41.55MB read
Requests/sec:   1903.26
Transfer/sec:    708.06KB

After

wrk --connections 10 --threads 10 --duration 60 'http://localhost:8080/?user_key=foo'
Running 1m test @ http://localhost:8080/?user_key=foo
  10 threads and 10 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency     5.41ms    1.07ms  21.00ms   72.87%
    Req/Sec   185.63     18.08   333.00     83.23%
  111020 requests in 1.00m, 40.33MB read
Requests/sec:   1847.49
Transfer/sec:    687.31KB

There is small difference, but we need to focus on performance later on.

@mikz mikz requested a review from davidor January 4, 2018 10:16
@mikz mikz force-pushed the post-action-phase branch 2 times, most recently from f8cd303 to 65436c0 Compare January 4, 2018 10:45
Copy link
Contributor

@davidor davidor left a comment

Choose a reason for hiding this comment

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

This solves a problem I had while implementing the cache policy 🏅

Can't comment on the lua's ffi part, as I'm not familiar with it yet. The rest looks good to me 👍

@davidor davidor mentioned this pull request Jan 9, 2018
@mikz mikz force-pushed the post-action-phase branch 3 times, most recently from 91c3e53 to efbf81c Compare January 12, 2018 08:14
this is needed to share ngx.ctx to subrequests or internal redirects

quite similar to https://github.com/tokers/lua-resty-ctxdump
implementation based on suggestions from openresty/lua-nginx-module#1057
sharing context to post_action enables executing the phase
with all context from the main request - including policy chain
* fix broken backtraces
* install all necessary tools
* ignore useless folders
@mikz mikz merged commit 486e466 into master Jan 12, 2018
@mikz mikz deleted the post-action-phase branch January 12, 2018 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants