Skip to content
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

Node::Advertise doesn't fail when advertising a duplicate service #217

Open
peci1 opened this issue Jan 29, 2021 · 2 comments
Open

Node::Advertise doesn't fail when advertising a duplicate service #217

peci1 opened this issue Jan 29, 2021 · 2 comments
Labels
documentation Improvements or additions to documentation good first issue Good for newcomers

Comments

@peci1
Copy link
Contributor

peci1 commented Jan 29, 2021

I'd expect that there can only be one "responser" for each advertised service. But that's not true.

Taking code from the examples:

$ ./responser_oneway &
[1] 10281
$ sleep 5 # to be sure the first responser gets properly advertised
$ ./responser_oneway &
[2] 10313

$ ign service -s /oneway -i
Service providers [Address, Request Message Type, Response Message Type]:
  tcp://172.17.0.1:39721, ignition.msgs.StringMsg, ignition.msgs.Empty
  tcp://172.17.0.1:43201, ignition.msgs.StringMsg, ignition.msgs.Empty

$ ign service -s /oneway --reqtype ignition.msgs.StringMsg --reptype ignition.msgs.Empty --timeout 2000 --req 'data: "Hello"'
Request received: [Hello]
Service call timed out

So it seems the discovery service is perfectly fine with having multiple providers of a service, but the client gets confused (the console only has one Request received... line, which means only one of the service responders got called). The timeout is weird.

I think it's wrong that the discovery service registers the other responser (but given the distributed nature of the system, maybe it' s inevitable?). However, Node::Advertise should definitely check for duplicate services before returning true.

@caguero
Copy link
Collaborator

caguero commented Jan 29, 2021

Hi @peci1 , this is a design decision. The discovery layer knows about all the services because in case one of them becomes unavailable, the requester node can use the next service up and running. If you have multiple instances of the same service, there's no guarantee of which one will provide the response. You can only assume that one of them will provide the response.

@peci1
Copy link
Contributor Author

peci1 commented Jan 29, 2021

Ah, then the docs are lacking, because I was searching really thoroughly and haven't found anything about this design decision. I searched both tutorials and source code of the library. I think such non-trivial decision needs to be described in the docs of Node::Advertise() then...

And the timeout shows to be unrelated to the two running responsers - it also happens with just one. It is probably caused by #97.

@chapulina chapulina added documentation Improvements or additions to documentation good first issue Good for newcomers labels Jan 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants