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

Balancer by lua stream #25

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
File renamed without changes.
82 changes: 82 additions & 0 deletions lib/ngx/balancer_stream.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
-- Copyright (C) Yichun Zhang (agentzh)
Copy link
Member

Choose a reason for hiding this comment

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

I think ngx.balancer.stream is a better module name than ngx.balancer_stream :)

Copy link
Author

Choose a reason for hiding this comment

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

Or perhaps ngx.stream.balancer and ngx.http.balancer ?

That way the namespace expands with other APIs ported.

Copy link
Member

Choose a reason for hiding this comment

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

Also, I hope we can still use the ngx.balancer module in the stream context. We could do a dynamic dispatch to ngx.balancer.stream in the top-level scope of ngx.balancer. My hunch is that we need the ngx.config.subsystem field in both ngx_http_lua and ngx_stream_lua first ;)

Copy link
Author

Choose a reason for hiding this comment

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

Yes, thats my intention. Once you, I or someone else implements the subsystem check.

Specifically I was planning on just:

if subsystem == "stream" then
return require("...")
else if ...
...

That way any overhead is limited to the initial loading.

Copy link
Member

Choose a reason for hiding this comment

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

@splitice It's already too late to introduce the ngx.http and ngx.stream namespaces. Also it fragments the API namespace unnecessarily. The hope is that most Lua modules can work automatically in both contexts.

Copy link
Author

Choose a reason for hiding this comment

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

Well I'll let you lay this out this however you like. I'll defer to your experience.



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


local C = ffi.C
local ffi_str = ffi.string
local errmsg = base.get_errmsg_ptr()
local FFI_OK = base.FFI_OK
local FFI_ERROR = base.FFI_ERROR
local int_out = ffi.new("int[1]")
local getfenv = getfenv
local error = error
local type = type
local tonumber = tonumber


ffi.cdef[[
int ngx_stream_lua_ffi_balancer_set_current_peer(ngx_stream_session_t *r,
const unsigned char *addr, size_t addr_len, int port, char **err);

int ngx_stream_lua_ffi_balancer_set_more_tries(ngx_stream_session_t *r,
int count, char **err);

int ngx_stream_lua_ffi_balancer_get_last_failure(ngx_stream_session_t *r,
int *status, char **err);
]]


local peer_state_names = {
[1] = "keepalive",
[2] = "next",
[4] = "failed",
}


local _M = { version = base.version }


function _M.set_current_peer(addr, port)
local r = getfenv(0).__ngx_sess
if not r then
return error("no request found")
end

if not port then
port = 0
elseif type(port) ~= "number" then
port = tonumber(port)
end

local rc = C.ngx_stream_lua_ffi_balancer_set_current_peer(r, addr, #addr,
port, errmsg)
if rc == FFI_OK then
return true
end

return nil, ffi_str(errmsg[0])
end


function _M.set_more_tries(count)
local r = getfenv(0).__ngx_sess
if not r then
return error("no request found")
end

local rc = C.ngx_stream_lua_ffi_balancer_set_more_tries(r, count, errmsg)
if rc == FFI_OK then
if errmsg[0] == nil then
return true
end
return true, ffi_str(errmsg[0]) -- return the warning
end

return nil, ffi_str(errmsg[0])
end


return _M
8 changes: 8 additions & 0 deletions lib/resty/core/base.lua
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,14 @@ if not pcall(ffi.typeof, "ngx_http_request_t") then
end


if not pcall(ffi.typeof, "ngx_stream_session_t") then
ffi.cdef[[
struct ngx_stream_session_s;
typedef struct ngx_stream_session_s ngx_stream_session_t;
]]
end


if not pcall(ffi.typeof, "ngx_http_lua_ffi_str_t") then
ffi.cdef[[
typedef struct {
Expand Down
8 changes: 5 additions & 3 deletions lib/resty/core/shdict.lua
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ ffi.cdef[[
int get_stale, int *is_stale);

int ngx_http_lua_ffi_shdict_incr(void *zone, const unsigned char *key,
size_t key_len, double *value, char **err);
size_t key_len, double *value, int exptime, char **err);
Copy link
Member

Choose a reason for hiding this comment

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

I'm afraid these changes are irrelevant and should be removed out of this PR.


int ngx_http_lua_ffi_shdict_store(void *zone, int op,
const unsigned char *key, size_t key_len, int value_type,
Expand Down Expand Up @@ -313,7 +313,7 @@ local function shdict_get_stale(zone, key)
end


local function shdict_incr(zone, key, value)
local function shdict_incr(zone, key, value, exptime)
zone = check_zone(zone)

if key == nil then
Expand All @@ -336,9 +336,11 @@ local function shdict_incr(zone, key, value)
value = tonumber(value)
end
num_value[0] = value

exptime = exptime or -1

local rc = C.ngx_http_lua_ffi_shdict_incr(zone, key, key_len, num_value,
errmsg)
exptime, errmsg)
if rc ~= 0 then -- ~= NGX_OK
return nil, ffi_str(errmsg[0])
end
Expand Down
24 changes: 24 additions & 0 deletions t/shdict.t
Original file line number Diff line number Diff line change
Expand Up @@ -915,3 +915,27 @@ qr/\[TRACE \d+ content_by_lua\(nginx\.conf:\d+\):7 loop\]/
-- NYI:
stitch




=== TEST 27: incr key expire
Copy link
Member

Choose a reason for hiding this comment

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

This change is unrelated to this PR, I'm afraid.

--- http_config eval: $::HttpConfig
--- config
location = /t {
content_by_lua '
local val, flags
local dogs = ngx.shared.dogs
local value, err = dogs:incr(nil, 32, 10)
if not ok then
ngx.say("failed to incr: ", err)
end
';
}
--- request
GET /t
--- response_body
failed to incr: nil key
--- no_error_log
[error]
[alert]
[crit]