eclipse-iceoryx / iceoryx

Eclipse iceoryx™ - true zero-copy inter-process-communication
https://iceoryx.io
Apache License 2.0
1.66k stars 388 forks source link

Refactoring of introspection #518

Open FerdinandSpitzschnueffler opened 3 years ago

FerdinandSpitzschnueffler commented 3 years ago

Brief feature description

The introspection needs a refactoring regarding:

ToDo:

Detailed information

When the introspection was implemented multiple pub/sub scenarios have not been considered, e.g. when a subscriber is subscribed to multiple publishers only one connection is displayed. When one publisher stops sending data then the subscriber is shown as disconnected (while still receiving data from the other publisher). This has to be adapted. Additionally, introspection tests are missing and the existing ones may have to be adapted once it is clear how data is internally stored and sent to the client.

Open questions:

budrus commented 3 years ago

When one publisher stops sending data then the subscriber is shown as disconnected (while still receiving data from the other publisher).

@FerdinandSpitzschnueffler . Is this because the intropspection processes the CaPro messages again to determine the connection state? And this "state machine" no does not fit to the mult-puhser one? I think we would have to change it in a way that state changes of subscribers are reported and not a redundant (and maybe other) processing of all CaPro messages is made

dkroenke commented 3 years ago

Do we want to stick to ncurses?

From my point of view we can stick to ncurses for the terminal output, but we need a solution for writing the introspection data into a parseable logfile for debugging purpose.

FerdinandSpitzschnueffler commented 3 years ago

When one publisher stops sending data then the subscriber is shown as disconnected (while still receiving data from the other publisher).

@FerdinandSpitzschnueffler . Is this because the intropspection processes the CaPro messages again to determine the connection state? And this "state machine" no does not fit to the mult-puhser one? I think we would have to change it in a way that state changes of subscribers are reported and not a redundant (and maybe other) processing of all CaPro messages is made

@budrus Yes, when one publisher stops sending data then the connection state is set to disconnected without checking whether the subscriber is also subscribed to another publisher.

MatthiasKillat commented 3 years ago

We need a proper state machine to track this, and in the right place (the introspection itself is the wrong place). It also can neither be done by the subscriber or publisher alone, it requires additional information. I coined those entities connections back then with the assumption that each subscriber has at most one possible connection (to a publisher that may or may not exist yet). This assumption is not true anymore and hence a careful rework is required.

MatthiasKillat commented 3 years ago

As for std::map replacement. Since the introspection does relatively frequent lookups, we need an efficient lookup mechanism, otherwise it is unacceptably slow when even a moderate number of ports are involved. I doubt a refactored lookup mechanism would work nicely without something like a map (something which provides logarithmic or even constant time lookup).

Now, creating a true map with efficent lookups with our constraints is not easy, but doable (essentially we need to implement red black trees without dynamic memory). A temporary solution can be a map-like interface (key value pairs etc.) but an inefficient search behind this interface which can be improved upon later. Thus we would not need a full blown map right away.

Note that it is perfectly doable to implement those trees but to lift these trees as a prototype (I have done so in the past), the time consuming part is the code beautification afterwards.

Note that to my knowledge cyclonedds stores such entity information in balanced trees (AVL I guess), and probably for good reason: it admits efficient lookup, insertion and deletion. In the long term I believe we need something similar.

elBoberido commented 3 years ago

Do we want to stick to ncurses?

From my point of view we can stick to ncurses for the terminal output, but we need a solution for writing the introspection data into a parseable logfile for debugging purpose.

I'd suggest to have a minimal introspection client dumping everything to the console and a fancy one with Python, Qt, HTML5, whatever and get rid of ncurses

dkroenke commented 3 years ago

Additional requirements: