Skip to content

@grpc/grpc-js & SO_REUSEPORT #1547

Closed
Closed

Description

@hugebdu

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.

Activity

  1. hugebdu commented on Aug 19, 2020

    @hugebdu
    ContributorAuthor

    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?

  2. murgatroid99 commented on Sep 2, 2020

    @murgatroid99
    Member

    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.

  3. hugebdu commented on Sep 3, 2020

    @hugebdu
    ContributorAuthor

    Hey Michael.

    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).

    WDYT?

  4. murgatroid99 commented on Sep 3, 2020

    @murgatroid99
    Member

    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.

  5. hugebdu commented on Sep 6, 2020

    @hugebdu
    ContributorAuthor

    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?

  6. murgatroid99 commented on Oct 28, 2020

    @murgatroid99
    Member

    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.

  7. hugebdu commented on Oct 29, 2020

    @hugebdu
    ContributorAuthor

    @murgatroid99 here's my PR #1614

    linter checks fail in posttest script, however it fails on master as well. probably the fix script glob should include subfolders?

  8. hugebdu commented on Oct 29, 2020

    @hugebdu
    ContributorAuthor

    @murgatroid99 thanks!
    IMO this one can be closed

  9. zyf0330 commented on Nov 12, 2020

    @zyf0330

    Hello, when does this feature SO_REUSEPORT release in @grpc/grpc-js?

  10. murgatroid99 commented on Nov 12, 2020

    @murgatroid99
    Member

    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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions