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

Enabling FastRTPS' ThroughputController #92

Closed
dhood opened this issue Feb 28, 2017 · 6 comments
Closed

Enabling FastRTPS' ThroughputController #92

dhood opened this issue Feb 28, 2017 · 6 comments
Labels
enhancement New feature or request
Milestone

Comments

@dhood
Copy link
Member

dhood commented Feb 28, 2017

This was originally discussed in #36
It can be enabled by un-commenting these lines

Motivation:

  • Enhance performance for sending large messages with "best effort" reliability (otherwise all message fragments are sent at the same time, something happens (yet to be determined what) that prevents data from being received, and as it's best effort the fragments are not resent)

Reasons against:

  • This can limit the throughput of systems.

Options:

  1. Enable only if best effort is being used. If users want higher throughput than the controller they will have to use reliable reliability.
  2. Enable always and accept the limited throughput.
  3. Expose a flow controller QoS setting, as it's also available in Connext see this page
  4. Expose the appropriate symbols to the user so that they can set the throughput controller themselves.

If we decide to go for 1 or 2, it may be useful to know that for the image_tools demo sending 320x240 @30fps we send ~8million bytes per second. A throughput controller of ~30,000 bytes/10ms (3million/s) is sufficient.

@dhood
Copy link
Member Author

dhood commented Feb 28, 2017

Thanks @mikaelarguedas for pointing out that implementing the throughput controller by un-commenting these lines will apply the same controller to all topics which will not be sufficient for cases where you need different controllers for different topics.

@mikaelarguedas
Copy link
Member

@dhood do you think that this can be closed in favor of ros2/rmw#115 ?

@mikaelarguedas mikaelarguedas added the more-information-needed Further information is required label Aug 5, 2017
@sloretz sloretz added enhancement New feature or request and removed more-information-needed Further information is required labels Mar 8, 2018
@sloretz
Copy link
Contributor

sloretz commented Mar 8, 2018

I removed more-information-needed because it seemed to be added in reference to ros2/rmw#115 which itself was closed in favor of more specific tickets, so this ticket shouldn't be closed in favor of that one.

It seems like the milestone for this could be untargeted - it is desirable but no one appears to be advocating for it to be resolve anytime soon.

@sloretz sloretz added this to the untargeted milestone Mar 8, 2018
@dirk-thomas dirk-thomas modified the milestones: untargeted, bouncy Mar 8, 2018
@dirk-thomas
Copy link
Member

I changed the milestone to Bouncy since I think it is crucial to fix the performance problem we are facing in e.g. the image demos with FastRTPS in the past releases.

@dhood dhood removed their assignment May 4, 2018
@mikaelarguedas mikaelarguedas added the ready Work is about to start (Kanban column) label May 7, 2018
@mikaelarguedas
Copy link
Member

With #202 (and this commit eProsima/Fast-DDS@201a393) it looks llike the performace issues we were facing with Fast-RTPS have been resolved. As the performance is now similar to the other RMW implementations I'm going to close this issue

@mikaelarguedas mikaelarguedas removed the ready Work is about to start (Kanban column) label Jun 18, 2018
@paulbovbel
Copy link

Just to verify since ros2/rmw#115 is closed as well, does this mean QoS settings for flow control will not be exposed via RMW?

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

No branches or pull requests

5 participants