-
Notifications
You must be signed in to change notification settings - Fork 62
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
Support the fd socket option when listening #78
Conversation
I still need to look this over fully but this is definitely ok and glad to have it. This is also used by services that use systemd's feature to get passed an open socket as fd 3. I'd also like to see unix domain socket support added, maybe I'll get that done soon. |
b54e04a
to
f362678
Compare
I hope I fixed the test suite issue. (I first had some troubles running the tests locally, but just managed to get them going. It turned out the port used in the test suite was occupied, by a locally running Jenkins of all things, haha :) Once I stopped that, all the weird ssl errors went away nicely) |
@@ -310,7 +320,7 @@ end_per_testcase(unary_no_auth, _Config) -> | |||
ok; | |||
end_per_testcase(multiple_servers, _Config) -> | |||
ok; | |||
end_per_testcase(unary_garbage_collect_streams, _Config) -> | |||
end_per_testcase(fd_socket_option, _Config) -> |
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.
Was about to merge this when I saw the end for testcase unary_garbage_collect_streams
was removed, I take it that was on accident? :) Can you add it back and I'll merge.
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.
Sorry, yes, absolutely accidental. Good catch, thanks. Fixed.
f362678
to
91e7fea
Compare
I should mention that I've noticed occasionally error logger printous about stealing control of fd= from output driver tcp_inet #Port, or something similar. I don't have the exact printout at hand at the moment, and fail to reproduce it locally, so this is from memory. There seems to be no ill effects, except for the printout itself, but maybe I ought to chase this a bit to see if it can be avoided? |
This PR allows the user to set the
{fd,Fd}
socket option when starting a grpc server. One use for this is if we want to open on any free port number, but may want to control the ip address. Another use case could be if the fd was opened by an external program with more privileges and sent to erlang usingsendmsg(2)
, for instance with fd_server.I'll try to describe our context a bit which led us to this proposal. We start grpc servers dynamically and, depending on our user, we may need to start several grpc servers. We may have to stick to some particular ip address, and we are not alone on the server, so we can often not bind to all addresses. But we can use any available port number. So we would ideally like to set the port to 0 to let the operating system assign us some available port. We have some means to communicate the dynamically selected port number to the user.
First we tried to set the
ip
option and theport => 0
option, but we faced a couple of other issues: one is that the port number is used as part of a process name and we would like to be able to open more than one grpc server for a particular ip address, another issue was that we would need to somehow get hold of the socket to learn what port number was actually assigned by the operating system.The fd option could solves both of these issues: we can open the listen socket in advance, use inet:sockname/1 and set the
ip
andport
options. Just gen_tcp need to be convinced to not try to bind, and this is what this PR does.Do you think this could be ok?