-
Notifications
You must be signed in to change notification settings - Fork 192
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
Added draft of an article about QoS #36
Conversation
I think this might be a good place to define our standard set of QoS profiles. |
@tfoote agreed. I'm adding a few to the proposed implementation and experimenting with them to have a sense of what they may look like. So far I have:
|
|
||
DDS provides fine-grained control over the Quality of Service (QoS) setting for each of the entities involved in the system. | ||
Common entities whose behavior can be modified via QoS settings include: Topic, DataReader, DataWriter, Publisher and Subscriber. | ||
QoS is enforced based on a Request vs Offerent Model, however Publications and Subscriptions will only match if the QoS settings are compatible. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unless I'm mistaken, in English I believe this is Offerer
rather than Offerent
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, thanks!
What's there looks good to me, but without more it's hard for me to see where this article is going. Maybe it would help me if you could add some more skeleton structure to the document with TODO's to fill it with details like the different proposed built-in QoS defaults. |
@wjwwood yeah, I just used this PR to structure my thoughts a bit. I'm making the API changes in the following PRs and will reflect them in this document: |
Both PrismTech OpenSplice and RTI Connext support loading of QoS policies via an external XML file. | ||
In environments where DDS is already deployed and also to enable more extensibility other than the offered by the ROS 2.0 and the predefined profiles, ROS 2.0 may provide loading of the QoS settings via the same mechanisms the underlying DDS implementations use. | ||
|
||
## References |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 for references!
While reviewing the realtime article, Adolfo encouraged me to cite sources inline. I like the idea of mapping information directly to sources, and I think we should come up with a unified way of doing it (if you want to do it in this article).
I see two options for this:
- Provide a link directly in the text
- Footnotes, for example: "DDS provides fine-grained control over QoS (1)", with a list of numbers at the bottom of the document, e.g. "1. Gordon A. Hunt, DDS - Advanced Tutorial, Using QoS to Solve Real-World Problems" and maybe with a page number for PDFs, etc.
|
||
## ROS 2.0 proposal | ||
|
||
Given the complexity of choosing the correct QoS settings for a given scenario, it may make sense for ROS 2.0 to provide a set of predefined QoS profiles for common usecases (e.g. sensor data, real time, etc.), while at the same time give the flexibility to control specific features of the QoS policies for the most common entities. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea of providing good defaults for different use cases, rather than a global set of defaults expected to be used everywhere. An integrator will be able to say "This topic is carrying range data; I shall use the sensor data QoS set."
What us the reasoning for the chosen depth sizes? And given the history settings, does this mean the dds layer us handling all message queuing, that there will not be a node level "callback queue"? |
@BobDean I think the current defaults are pretty arbitrary, and @esteve is looking into the best defaults for a few cases. DDS is handling all of the queueing, though we might have a ring buffer, whose size is driven by the depth setting, which is used for intra process communications. That's all still up in the air at the moment though. |
Now that the QoS API has been designed and iterated over, it seems like a good time to make this design document more specific so that future changes we make are consistent with each other. Here are a few questions that have been raised about the current prototype during the review process:
|
@jacquelinekay thanks for summarizing the discussion, I'll update the document to address these questions. |
I've added a section about QoS profiles. I don't know how intraprocess should honor (or if it should) the QoS settings, so I added that as an open question. Does anyone think it'd worth adding a terminology section explaining every QoS setting implemented in the current API? I think it'd be useful, but I worried this would make the article too long. |
Terminology might be good, but I'd suggest linking to a DDS documentation of QoS definitions, just clearly state which ones we're supporting maybe? |
I actually think this article should stand on it's own and that readers should not have to go to a set of DDS documentation to understand what QoS settings ROS is providing and what they do. |
I agree with @wjwwood , I prefer documentation that doesn't force me to jump to external resource for core topics. I can add links to the QoS section of the DDS spec in case the readers want to know more. |
I updated the document removing the QoS profile and instead explained it can store and how the values affect its behavior. |
* Reliable: guarantee that samples are delivered, may retry multiple times. | ||
* Durability. | ||
* Transient local: *only applies to `DataWriter`s*. `DataWriter` becomes responsible of persisting samples until a corresponding `DataReader` becomes available. | ||
* Volatile: no attempt is made to persist samples. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For each of the main bullets there is also the option of "system default".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added note about this. Thanks.
a7fd04f
to
ce36d2f
Compare
I've rebased this branch and squashed the commits. Is there anything that needs improving? |
|
||
* Default QoS settings for publishers and subscriptions. | ||
In order to make the transition from ROS1 to ROS2, exercising a similar network behavior is desirable. | ||
By default, publishers and subscriptions are reliable in ROS2. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add note here that this default value won't work for larger messages for all rmw implementations atm?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, will update the document to reflect this.
Addressed @dirk-thomas 's latest feedback, squashed again and pushed. |
* Default QoS settings for publishers and subscriptions. | ||
In order to make the transition from ROS1 to ROS2, exercising a similar network behavior is desirable. | ||
By default, publishers and subscriptions are reliable in ROS2. | ||
However, many DDS vendors don't support publishing large messages (e.g. images) with reliable connections. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sentence is a bit misleading. Connext does support publishing large messages with reliable connections. But we have to use a different API for that.
Todo:
|
Updated the article with the most recent feedback. I'm going to squash and merge this PR soon unless there are more comments. |
Currently the article does not mention the parameter related profiles. Is that intentional? If not, I think especially the depth of 1000 could use some rational. |
@esteve Can you please comment on the previous question. |
The parameters profile is not mentioned in the article because we still haven't officially released the support for parameters. In any case, the value in there is purely arbitrary, the parameters profile has a priori the same rationale as the services one, so it may make sense to use the same values. |
Since the parameters are scheduled for the upcoming alpha next week I think the article should be updated to cover the QoS setting for services (and parameters). |
Draft of an article about QoS
Comments and questions are more than welcome.