Balancer by lua stream #25
Conversation
| @@ -0,0 +1,82 @@ | |||
| -- Copyright (C) Yichun Zhang (agentzh) | |||
There was a problem hiding this comment.
I think ngx.balancer.stream is a better module name than ngx.balancer_stream :)
There was a problem hiding this comment.
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.
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 ;)
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
@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.
There was a problem hiding this comment.
Well I'll let you lay this out this however you like. I'll defer to your experience.
|
@splitice Just a side note: I think the |
|
Currently I have no use for the semaphore module so wont be doing it myself at this stage. I ported this because of the lack of variable support in the stream subsystem preventing me from doing something as simple as If you have any comments on any of the PR's I've made feel free to let me know today. I am spending the day porting over a couple more of our small patches to the stream subsystem so its not out of my way. |
| @@ -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.
I'm afraid these changes are irrelevant and should be removed out of this PR.
|
@splitice Will you add a test file for using |
|
Probably best you do. I can do a theoretical port but dont have the time to get the environment passing. |
|
Also might be best to check if the balancer_by_lua_* tests all pass first :) |
|
@splitice Please check out this tutorial for how to run the tests on your side: https://openresty.gitbooks.io/programming-openresty/content/testing/ It should be easy :) |
|
This branch needs to rebase to the latest git master before it can be merged. |
|
|
||
|
|
||
|
|
||
| === TEST 27: incr key expire |
There was a problem hiding this comment.
This change is unrelated to this PR, I'm afraid.
Commits in this branch correspond to the changes for Balancer by LUA in the Stream Subsystem (
openresty/stream-lua-nginx-module#7).
I have renambe the balancer.lua to balancer_http, and created the new implementation as balancer_stream.lua.
I will create a balancer.lua compatibility wrapper once the subsystem check is implemented.
Feel free to adjust the API to match your desired layout.