yarpmanager and the curse of "localhost"

[Nicogene]

As pointed out by @claudiofantacci, if localhost is specified in the xml file, it is no longer possible to modify the host from yarpmanager, and on the other hand, if you change the "host" field from the gui and you type "localhost" it says that there are not any yarprun --server /localhost.

@apaikan it is an desired behavior?
Otherwise I will work for fixing it :wink:

[apaikan]

hi guys,
this is an old issue, many times discussed and it has been left as it is up to now.

where is the origin of the problem? well, yarpmanager has two distinct brokers: localborker and yarprun. after parsing the xml file, yarpmanager should decide which broker to instantiate and initialize. so, if u change the hostname from localhost to yarprun, then u need to reinstantiate the coresponding broker and restart the initialization steps.
it is indeed doable but ensure you have reinitialised all the steps carefully.
in bocca al lupo :)

[drdanz]

The easiest fix would be to reject the string "localhost", unless it is set on the xml file

[claudiofantacci]

The easiest fix would be to reject the string "localhost", unless it is set on the xml file

What do you mean with reject? That if I specify, say, _icub21_ as node and I try to change it to _localhost_ it will not be allowed?

Anyway, I was just wondering with @Nicogene whether it was possible, and not too much complicated, to allow specifying either a legit node name _or_ localhost without restrictions.

It seems however that it's a bit complicated and maybe it does not make much sense...is it?

[traversaro]

Maybe I am missing something, but if our problem is to express clearly to the user what is going on, perhaps a possible solution is just to extend the XML specification and the gui to explicitly express the broker used for launching the process.
If the broker is not present in the XML file, its default value would be localhost if the host is localhost, and yarprun if is any other string, preserving the current behaviour.
If this broker can be changed in an already opened application is a different problem, but if that is possible, the relative gui element could be a multiple choice interface like we have for carriers.

[claudiofantacci]

I wouldn't say that there is something wrong in general.
I just discussed with @Nicogene about this scenario:

• if XML has _localhost_, you can't change it manually on the GUI
• if XML has _node name_, you can change it manually on the GUI, but _localhost_ won't work

then I was just wondering:

• can we allow manual specification of node names in the GUI?

Now I understand the problema bout the broker, so the solution might not be trivial.

[drdanz]

The easiest fix would be to reject the string "localhost", unless it is set on the xml file

What do you mean with reject? That if I specify, say, _icub21_ as node and I try to change it to _localhost_ it will not be allowed?

Exactly. Unless we modify it to change the broker when you change it from/to localhost, but this will require more work.
At the moment if you change it to "localhost" it will not work, and you will not be able to change it back, unless you close and reopen the application.
Rejecting "localhost" is in my opinion a first improvement from a broken behaviour to a debatable but working behaviour.

[claudiofantacci]

Rejecting "localhost" is in my opinion a first improvement from a broken behaviour to a debatable but working behaviour.

I agree! I'll discuss this with @Nicogene.

[Nicogene]

I would try to instantiate both broker for each module in an application and switch to the local broker if in the entry of host there is localhost.
I have no idea about the feasibility and if it will get worse the performances... but I could try and see 😉

[apaikan]

@nicogene
If later we need to add more broker such as 'ssh', 'roslunch'? then practically we must instantiate all of them? :D

[claudiofantacci]

@Nicogene
If later we need to add more broker such as 'ssh', 'roslunch'? then practically we must instantiate all of them? :D

I'm not an expert of brokers, are they that heavy to instantiate? Anyway, I understand that it may be more clean to have around the only needed brokers.

If in the long run we will support that much brokers, than it may be better to think and design a flexible architecture to allow different brokers instantiation, swith and a way to add them in a scalable way.

@apaikan what's your point of view?

[apaikan]

@Nicogene @claudiofantacci @drdanz

Actually it is flexible enough. Bellow, I have created a simplified class diagram to show how brokers have been used in yarpmanager. Please pay attention to 'composition' and 'aggregation' arrows to grasp the ownership of each instance. The steps you need to do when switching to a new broker (e.g. from LocalBroker to YarpBroker are

1. ask Executable to delete the old broker (indeed stopping module before deleting the broker)
2. create a proper broker using createBroker()
3. implement a new method to Executable to setup the new broker as it is done in its constructor!
4. pay attention to the watchdog flag! Basically if watchdog is enabled, Executable uses a separate thread to continuously monitor the execution of the module and in case of the failure, it calls the corresponding methods from Event, e.g. onExecutableDied().

Each Executable has an state machine inside. this state machine implement the life cycle of each underlying module such as '_Ready_', '_Suspended_', '_Running_', '_Connecting_', etc. Take a look at execstate.h. (@Nicogene ormai sei un monstro di state machine 👍)

P.S. I hope you find this comment useful. As soon i can find the complete class diagram of the yarpmanager somewhere in my back files, I'll share with you guys! :)

[claudiofantacci]

Super awesome @apaikan 😎 . It's quite clear to me, @Nicogene will for sure understand how to handle this 👍

[Nicogene]

Thanks @apaikan :kissing_heart:
As soon as I have some time I will investigate it, the fast fix is, as suggested by @drdanz, to reject the "localhost" entry from the gui.
The real fix is not trivial so I need more time to think about it 😉

[Nicogene]

In #1487 there is a first draft for fixing this issue.. on my machine works the switch between localhost to yarprun but I have to do more tests

[Nicogene]

Closed via #1487

[claudiofantacci]

Awesome! Thanks @Nicogene and @apaikan !