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

ngx.ctx inheirt #1057

Closed
tokers opened this issue May 3, 2017 · 13 comments
Closed

ngx.ctx inheirt #1057

tokers opened this issue May 3, 2017 · 13 comments

Comments

@tokers
Copy link
Contributor

tokers commented May 3, 2017

We use the magic ngx.ctx for storing something, but when internel redirect is happening, ngx.ctx will be dereferenced and the gc will recycle it, so most of the time we have to use ngx.var.VARIABLE, but the cost is expensive especially when the QPS is high. My problem is, why not add a flag to control whether inherits the origin ngx.ctx when ngx.exec is invoked. 😃

@agentzh
Copy link
Member

agentzh commented May 3, 2017

@tokers The problem is that the nginx core clears all modules' ctx data upon internal redirects. We'll need special hacks to work around that.

@tokers
Copy link
Contributor Author

tokers commented May 3, 2017

@agentzh Thanks for your reply! Is there any other compromise way which is better than the ngx.var.VARIABLE .

@agentzh
Copy link
Member

agentzh commented May 3, 2017

@tokers I think maybe we can abuse a special custom nginx variable created by this module under the hood (on the C land) and use it to hold the pointer to ngx.ctx, for example. Mind to submit a patch?

@tokers
Copy link
Contributor Author

tokers commented May 3, 2017

@agentzh I will try!

@tokers
Copy link
Contributor Author

tokers commented May 3, 2017

@agentzh I hacked the limits of ngx.ctx when ngx.exec is invoking on the Lua hand.
Here is the demo.

ngx_ctx_bypassing.lua

local debug = require "debug"
local ffi = require "ffi"
local base = require "resty.core.base"

local C = ffi.C
local registry = debug.getregistry()
local tostring = tostring
local tonumber = tonumber
local _M = {}

ffi.cdef[[
int ngx_http_lua_ffi_set_ctx_ref(ngx_http_request_t *r, int ref);
]]

function _M.stash_ngx_ctx()
      local ctxs = registry.ngx_lua_ctx_tables
      local ctx_ref = base.ref_in_table(ctxs, ngx.ctx)
      local r = getfenv(0).__ngx_req
 
      if not r then
          return error("no request found")
      end
 
      if C.ngx_http_lua_ffi_set_ctx_ref(r, ctx_ref) ~= base.FFI_OK then
          ngx.log(ngx.DEBUG, "stash the old ngx.ctx failed")
      end
 
     ngx.var.ctx_ref = tostring(ctx_ref)
end

function _M.apply_ngx_ctx()
      local ctx_ref = tonumber(ngx.var.ctx_ref)
      if not ctx_ref then
          return
      end
 
      local ctxs = registry.ngx_lua_ctx_tables
      local origin_ngx_ctx = ctxs[ctx_ref]
      ngx.ctx = origin_ngx_ctx
 
      local FREE_LIST_REF = 0
      ctxs[ctx_ref] = ctxs[FREE_LIST_REF]
      ctxs[FREE_LIST_REF] = ctx_ref
      ngx.var.ctx_ref = ""
end

return _M

location conf

location /t1 {
    content_by_lua_block {
         local bypass_ngx_ctx = require "ngx_ctx_bypassing"
         ngx.ctx = {
             Date = "Wed May  3 15:18:04 CST 2017",
             Site = "unknown"
        }
        bypass_ngx_ctx.stash_ngx_ctx()
        ngx.exec("/t2")
    }
}

location /t2 {
    content_by_lua_block {
         local bypass_ngx_ctx = require "ngx_ctx_bypassing"
         ngx.ctx = {
             Date = "Wed May  3 15:18:04 CST 2017",
             Site = "unknown"
        }
        bypass_ngx_ctx.apply_ngx_ctx()
        ngx.say("Date: " .. ngx.ctx["Date"] .. " Site: " .. ngx.ctx["Site"])
    }
}

My lua-nginx-module version is v0.10.7, Nginx version is 1.11.2, lua-resty-core version is v0.1.9.
It works well on my local machine. Is there anything improper?

@bjoe2k4
Copy link
Contributor

bjoe2k4 commented May 3, 2017

Would be highly interested in this option as well.

@agentzh
Copy link
Member

agentzh commented May 3, 2017

@tokers Yeah, your workaround looks good to me. I didn't realize it can be done all in Lua. This is clever.

@bjoe2k4
Copy link
Contributor

bjoe2k4 commented May 3, 2017

@tokers I think the definition of $ctx_ref is missing form your example. Some global switch to turn this on without having to do this manually every time using ngx.exec and set the variable in every server {} directive would be appreciatied.
For example i am doing some logging with log_by_lua in an error_page directive (triggers internal redirect), collecting some data from ngx.ctx.

@tokers
Copy link
Contributor Author

tokers commented May 4, 2017

@bjoe2k4 Yes, i miss the definition of $ctx_ref, sorry!

@tokers
Copy link
Contributor Author

tokers commented May 4, 2017

@agentzh @bjoe2k4 , the function stash_ngx_ctx in my demo has a bug, we need to remove the line about C.ngx_http_lua_ffi_set_ctx_ref, it will cover the original ctx_ref in the ngx_http_lua_ctx_t.

@agentzh
Copy link
Member

agentzh commented May 4, 2017

@tokers Yeah, we should not override the existing ctx table ref. We just need to anchor the ngx.ctx table somewhere (like in a custom lrucache store to avoid potential memory leaks).

@spacewander
Copy link
Member

Maybe we could introduce get_ctx_ref and set_ctx_ref APIs, and let the caller decide where to store the reference.

@agentzh
Copy link
Member

agentzh commented May 5, 2017

@spacewander Frankly I don't really want to expose such implementation details to the user API level. It makes future implementation changes much harder if not impossible.

@tokers tokers closed this as completed May 7, 2017
mikz added a commit to 3scale/APIcast that referenced this issue Dec 22, 2017
need to share context between internal redirect
(openresty/lua-nginx-module#1057)

but then the policies have total control over generating the content
so they can use cosockets / files / upstream / generate content

please note this is higly experimental and should not be merged to
master yet
mikz added a commit to 3scale/APIcast that referenced this issue Dec 22, 2017
need to share context between internal redirect
(openresty/lua-nginx-module#1057)

but then the policies have total control over generating the content
so they can use cosockets / files / upstream / generate content

please note this is higly experimental and should not be merged to
master yet
mikz added a commit to 3scale/APIcast that referenced this issue Dec 22, 2017
need to share context between internal redirect
(openresty/lua-nginx-module#1057)

but then the policies have total control over generating the content
so they can use cosockets / files / upstream / generate content

please note this is higly experimental and should not be merged to
master yet
mikz added a commit to 3scale/APIcast that referenced this issue Jan 4, 2018
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
mikz added a commit to 3scale/APIcast that referenced this issue Jan 4, 2018
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
mikz added a commit to 3scale/APIcast that referenced this issue Jan 11, 2018
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
mikz added a commit to 3scale/APIcast that referenced this issue Jan 12, 2018
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
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

No branches or pull requests

4 participants