-
Notifications
You must be signed in to change notification settings - Fork 275
Balancer by lua stream #25
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
Open
splitice
wants to merge
5
commits into
openresty:master
Choose a base branch
from
splitice:balancer-by-lua-stream
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
1ff2dea
shared dictionary incr method exptime added to FFI api
splitice ff6287f
add simple shared dictionary test
splitice beea2ea
Merge remote-tracking branch 'refs/remotes/openresty/master'
splitice d78c8a3
initial creation of two balancer modules
splitice ce47583
add missing typedef
splitice File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
File renamed without changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,82 @@ | ||
-- Copyright (C) Yichun Zhang (agentzh) | ||
|
||
|
||
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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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, | ||
|
@@ -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 | ||
|
@@ -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 | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -915,3 +915,27 @@ qr/\[TRACE \d+ content_by_lua\(nginx\.conf:\d+\):7 loop\]/ | |
-- NYI: | ||
stitch | ||
|
||
|
||
|
||
|
||
=== TEST 27: incr key expire | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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] |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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 thanngx.balancer_stream
:)There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 tongx.balancer.stream
in the top-level scope ofngx.balancer
. My hunch is that we need thengx.config.subsystem
field in both ngx_http_lua and ngx_stream_lua first ;)There was a problem hiding this comment.
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:
That way any overhead is limited to the initial loading.
There was a problem hiding this comment.
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
andngx.stream
namespaces. Also it fragments the API namespace unnecessarily. The hope is that most Lua modules can work automatically in both contexts.There was a problem hiding this comment.
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.