-
Notifications
You must be signed in to change notification settings - Fork 2.1k
add a function for get socket fd #1147
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
Conversation
@mlboy Will you please explain your use cases for this API? |
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.
Exposing fd directly may break the internal I/O scheduler for cosockets since it breaks the low level I/O encapsulation. So I wonder why you need this in the first place.
src/ngx_http_lua_socket_tcp.c
Outdated
ngx_http_lua_socket_tcp_getsockfd(lua_State *L) | ||
{ | ||
ngx_connection_t *c; | ||
ngx_http_lua_socket_tcp_upstream_t *u; |
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.
Style: variable identifiers must be vertically aligned up. See other similar places for examples.
src/ngx_http_lua_socket_tcp.c
Outdated
} | ||
|
||
c = u->peer.connection; | ||
lua_pushinteger(L,(int) c->fd); |
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.
Style: need a space after the comma character.
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, need to test if the fd is -1 before returning it.
@@ -782,6 +786,29 @@ ngx_http_lua_socket_tcp_connect(lua_State *L) | |||
return lua_yield(L, 0); | |||
} | |||
|
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.
Style: we use 2 blank lines to separate successive C function definitions. See other similar places for examples.
lua_pushinteger(L,(int) c->fd); | ||
|
||
return 1; | ||
} | ||
|
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.
Ditto.
src/ngx_http_lua_socket_tcp.c
Outdated
@@ -265,6 +266,9 @@ ngx_http_lua_inject_socket_tcp_api(ngx_log_t *log, lua_State *L) | |||
lua_pushcfunction(L, ngx_http_lua_socket_tcp_settimeouts); | |||
lua_setfield(L, -2, "settimeouts"); /* ngx socket mt */ | |||
|
|||
lua_pushcfunction(L, ngx_http_lua_socket_tcp_getsockfd); | |||
lua_setfield(L, -2, "getsockfd"); /* ngx socket mt */ |
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.
The sock
word in the method name is redundant. It cannot really be something else. It can only be called on cosocket objects anyway. Or are you following LuaSocket's Lua API here? (I couldn't find such an API in LuaSocket).
step 2 i will add a method like sock:send_fd(fd,...) |
@mlboy Sending fds across process boundary can be tricky in that you would also have to pass all the associated state over and rebuild all the data structures in the other process. All this work may incur significant overhead and is very tricky to get exactly right given the complexity of the problem. For your use case, I think it's better to just pass "control flow" and "data" across the process boundary instead of passing fds and cosockets. We already have production users who use ngx.semaphore + lua_shared_dict + ngx.sleep to do such kind of things, achieving 1.5 million ~ 2 million connections in a single box. We could make this approach even more efficient when we add listen() and accept() methods to the cosocket objects so that we can sync data and control flow on the nginx worker process level more efficiently than the lua_shared_dict + ngx.sleep approach (the intra-worker level request dispatch is already very efficient with ngx.semaphore). |
f924579
to
fef2581
Compare
This pull request is now in conflict :( |
If anyone needs this API, should reimplement it via ffi. |
I’d like to reimplement this via FFI, but I’m not sure about the best approach within the lua-nginx-module. Are there any existing examples or recommendations on how to properly work using FFI in this context? I need If you have any guidance or references, I’d really appreciate it. |
here is an example: ngx_http_lua_ffi_socket_tcp_sslhandshake |
Hi, are you still working on this? I share the same need. |
@linsite https://github.com/gustavosbarreto/websocket-fd-handoff |
Thanks a lot. |
Have added the getfd() function. |
I hereby granted the copyright of the changes in this pull request
to the authors of this lua-nginx-module project.