You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Is your feature request related to a problem? Please describe.
Another discrepancy with grpc-native we found on linux is inability to bind multiple grpc.Server instances on the same port in @grpc/grpc-js.
We have a requirement to load/unload services without downtime and since both implementations do not allow that what we did was bringing up a new instance of server with the new set of services and gracefully shutting down the previous instance. Worked like magic with grpc-native.
Didn't dive too deep but it looks like the problem goes all the way to node's http2 and even nghttp2. At least I couldn't find any mentions of that option.
UPD
apparently it looks like this limitation is here for historical reasons. I just removed those lines and successfully mutated handlers map (via handlers.clear() and addService(...)) without restart.
have all my contract tests passing.
To sum up - any chance you will consider runtime mutation APIs or should I stay with my monkey patching on my own risk?
I'm sorry, I meant to reply to this earlier. As far as I can tell, there was never a strong reason to have that restriction on registering services after starting the server, especially on Node, where there isn't any significant concern about synchronizing between mutating the registered handlers map and dispatching incoming requests. So, I would be OK with removing that restriction entirely.
Removing services is a little trickier because there isn't an existing API for that, so we'll have to add one. The handlers member is private, so modifying it directly isn't the right solution.
Also, regarding the original question about SO_REUSEPORT, Node does not provide any JavaScript API to set that option. That's less about nghttp2, and more about Node's net built in module.
I guess there could be Server#unregister(name: string, ....) - the counterpart of Server#register(...).
On top of that we can have Server#removeService(...) that accepts either instance of ServiceDefinition or some handle returned by addService (requires changing the return type of addService).
I think it can just be Server#unregister(name:string) and Server#removeService(service: ServiceDefinition). You can't register the same name/path more than once, so the name uniquely identifies a registered handler, and a service uniquely identifies a list of registered paths.
Sounds great. I can try and make a PR. Just need to make sure I understand all the tests setup.
Is it enough to add new tests only to the @grpc-js or you expect also new tests being added to the cross implementation tests?
I'm sorry for the silence here. I thought I responded to the last comment. If you're still interested, tests for this would just need to go in the grpc-js test directory because we would not want to replicate this change for the other implementation.
The title doesn't exactly match what this issue ended up being about. It is primarily a request for the ability to add and remove services from a running server, and that is implemented in the latest release 1.2.0.
Activity
hugebdu commentedon Aug 19, 2020
UPD
apparently it looks like this limitation is here for historical reasons. I just removed those lines and successfully mutated handlers map (via
handlers.clear()andaddService(...)) without restart.have all my contract tests passing.
To sum up - any chance you will consider runtime mutation APIs or should I stay with my monkey patching on my own risk?
murgatroid99 commentedon Sep 2, 2020
I'm sorry, I meant to reply to this earlier. As far as I can tell, there was never a strong reason to have that restriction on registering services after starting the server, especially on Node, where there isn't any significant concern about synchronizing between mutating the registered handlers map and dispatching incoming requests. So, I would be OK with removing that restriction entirely.
Removing services is a little trickier because there isn't an existing API for that, so we'll have to add one. The
handlersmember is private, so modifying it directly isn't the right solution.Also, regarding the original question about SO_REUSEPORT, Node does not provide any JavaScript API to set that option. That's less about nghttp2, and more about Node's
netbuilt in module.hugebdu commentedon Sep 3, 2020
Hey Michael.
I guess there could be
Server#unregister(name: string, ....)- the counterpart ofServer#register(...).On top of that we can have
Server#removeService(...)that accepts either instance ofServiceDefinitionor somehandlereturned byaddService(requires changing the return type ofaddService).WDYT?
murgatroid99 commentedon Sep 3, 2020
I think it can just be
Server#unregister(name:string)andServer#removeService(service: ServiceDefinition). You can't register the same name/path more than once, so the name uniquely identifies a registered handler, and a service uniquely identifies a list of registered paths.hugebdu commentedon Sep 6, 2020
Sounds great. I can try and make a PR. Just need to make sure I understand all the tests setup.
Is it enough to add new tests only to the
@grpc-jsor you expect also new tests being added to the cross implementation tests?murgatroid99 commentedon Oct 28, 2020
I'm sorry for the silence here. I thought I responded to the last comment. If you're still interested, tests for this would just need to go in the
grpc-jstest directory because we would not want to replicate this change for the other implementation.hugebdu commentedon Oct 29, 2020
@murgatroid99 here's my PR #1614
linter checks fail in
posttestscript, however it fails on master as well. probably the fix script glob should include subfolders?hugebdu commentedon Oct 29, 2020
@murgatroid99 thanks!
IMO this one can be closed
zyf0330 commentedon Nov 12, 2020
Hello, when does this feature SO_REUSEPORT release in @grpc/grpc-js?
murgatroid99 commentedon Nov 12, 2020
The title doesn't exactly match what this issue ended up being about. It is primarily a request for the ability to add and remove services from a running server, and that is implemented in the latest release 1.2.0.