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

Implement latched topics on TCP #2

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

peci1
Copy link
Contributor

@peci1 peci1 commented Oct 29, 2016

Transmitting a latched topic over the nimbro relay was not supported.

This PR adds several features:

  • The publisher on the receiving side will have the "latched" option set if the topic is marked as latched in the YAML config.
  • Each TCP sender provides a service (over nimbro) which, when called, republishes latched messages to all latched topics. This makes sure that receivers started after the sender will get even the first latched message.
  • The above mentioned service can be called automatically when the receiver is ready.

This PR almost surely breaks ABI, if take care about it. On the other hand, API changes are not breaking compatibility with previous versions.

@xqms
Copy link
Member

xqms commented Oct 29, 2016

Thanks for the PR!

If I understand correctly, your are supporting latching only for the TCP transport. In that case, I do not understand the need for the complex "latch helper" setup. I would suggest to re-send latched messages whenever TCPSender::connect() is successful, indicating a new receiver has started.

@peci1
Copy link
Contributor Author

peci1 commented Oct 29, 2016

Oh man, could it be that simple? I admit I haven't understood the whole relay code, but it looked impossible to me to tell when the client has connected. But as you pointed me to the right direction, it now seems clear to me.

Few questions arise:

  1. Isn't there a possible infinite loop if I resend the latched messages from connect, which is itself called from the send method? I mean in case the connection is very unreliable or so...
  2. Now, when it's almost clear the whole cumbersome thing was not needed, is there a chance to make use of it on the UDP transport?

@xqms
Copy link
Member

xqms commented Oct 29, 2016

Isn't there a possible infinite loop if I resend the latched messages from connect, which is itself called from the send method? I mean in case the connection is very unreliable or so...

I guess so. At the very least we should prevent the inner send() routine (resending a latched message) from calling connect() again. Maybe an additional parameter reconnect with default value true?

Now, when it's almost clear the whole cumbersome thing was not needed, is there a chance to make use of it on the UDP transport?

I have never seen a use case for latched topics over UDP. Typically, UDP data is streaming data, where latching makes no sense. Or do you have an actual use case for that?

@xqms
Copy link
Member

xqms commented Oct 29, 2016

Ah, and there is one more issue: If there are no messages to transmit, send will never get called, and will never notice the dropped connection. So we should introduce a timer which checks whether the connection is still open, and if not, reconnects (and re-sends latched messages).

@peci1
Copy link
Contributor Author

peci1 commented Oct 29, 2016

Ok, I'm trying to rewrite and will send a new PR in a while. Your anti-looping suggestion sounds good to me.

And regarding latched UDP topics - it probably really doesn't make sense to latch UDP topics... Should I add a warning to the UDP transport when it finds a topic which requires latching?

@peci1
Copy link
Contributor Author

peci1 commented Oct 29, 2016

Well, now I think if this wasn't my original issue... I kind of get lost thinking about all the possible cases. Wouldn't periodical connection check be a problem? Either performance-wise, or just for the case that the receiver is stopped and is not mentioned to be re-run? (e.g. unsubscribing from all topics on the receiving side).

@peci1
Copy link
Contributor Author

peci1 commented Oct 29, 2016

Wait, is the dropped connection really a problem?

The receiver also sets the "latched" flag at the publishers it creates. So even if the connection drops, the subscribers on the receiving side will get properly handled. Or do I miss some other important point?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants