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

Update "About Quality of Service settings" Wiki Page #661

Merged
merged 5 commits into from
May 11, 2020

Conversation

mm318
Copy link
Member

@mm318 mm318 commented May 7, 2020

Closes #280.

The current "About Quality of Service settings" wiki page is quite out of date; it is missing some of the new QoS policies that were added, missing some new concepts, and using some inconsistent terminology.

mm318 added 2 commits May 7, 2020 10:14
Signed-off-by: Miaofei <miaofei@amazon.com>
Signed-off-by: Miaofei <miaofei@amazon.com>
Copy link
Contributor

@emersonknapp emersonknapp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a huge improvement already, just a few clarifying points I'd like us to cover

Note however that even the reliable policy in ROS 2 is implemented using UDP, which allows for multicasting if appropriate.

The durability policy combined with a depth of 1 provides functionality similar to that of "latching" subscribers.
The “durability” policy combined with a depth of 1 provides functionality similar to that of “latching” subscribers.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The “durability” policy combined with a depth of 1 provides functionality similar to that of “latching” subscribers.
The “durability” policy "transient local", combined with any depth, provides functionality similar to that of “latching” publishers.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Subscriptions *request* a QoS profile that is the “minimum quality” that it is willing to accept, and publishers *offer* a QoS profile that is the “maximum quality” that it is able to provide.

Connections are only made if the every policy of the requested QoS profile is not more stringent than that of the offered QoS profile.
The less strict of the two policies will be the one used for the connection, and it is referred to as the “actual QoS” policy.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably mention that all connections coming out of a publisher don't necessarily have the same QoS - meaning the publisher has a maximum offer, but can form differing connections based on requests.

I saw at one point the misconception that when you requested a "different" qos profile from a publisher, that it somehow "switched qos profiles", which isn't quite right conceptually - the publisher's offer stays the same.

I just realized that "actual QoS" is a different concept in the rmw API and just seems to mean "whatever SYSTEM_DEFAULT resolved to" - we should probably remove that. https://github.com/ros2/rmw/blob/master/rmw/include/rmw/rmw.h#L660

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, it may be more precise to say "the subscription's request, if compatible with the offer, will be the qos profile of the connection" - since it will always be the less strict if the request is compatible.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am now hesitant on the pushing the idea of "QoS profile of the connection". It gives off a connotation that there is a synchronization of behavior between the publisher and subscription. And if there are multiple subscriptions with different QoS profiles, there would be confusion as to which subscription's QoS profile would the publisher synchronize its behavior.

I did some experimentation and found that there is no change in behavior. If the publisher's offered deadline period is x and the matched subscription's requested deadline period is 2x, their deadline missed callbacks are still triggered every x and 2x intervals respectively. Then what is the point of the "QoS profile of the connection" (granted that although a deadline missed callback triggering at every x is still valid behavior for a QoS profile whose deadline period is 2x, it would need an unnecessarily verbose explanation)? I think we can just stop at whether a connection will be made or not.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok - feels like we dive into implementation details, we can avoid making any hard statements about the exact behavior, just that a connection is made if it's compatible

- Connection
- Result
- Actual QoS Policy
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the re-realization about the "actual" terminology - we should switch this back to "Resulting Connection" or something

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.


The following are the possible QoS events that developers may subscribe to:

* Offered deadline missed
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a little unclear here which callbacks apply to which objects, maybe split this out into a "Subcription Callbacks" and "Publisher Callbacks" section? Are there callbacks that are common to both, or are there just matching pairs for each?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are just matching pairs, although the callbacks of each pair are very similar to each other.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have split the list into two separate lists, one for publisher callbacks and one for subscription callbacks.

QoS profile compatibility is determined based on a “Request vs Offered” model.
Subscriptions *request* a QoS profile that is the “minimum quality” that it is willing to accept, and publishers *offer* a QoS profile that is the “maximum quality” that it is able to provide.

Connections are only made if the every policy of the requested QoS profile is not more stringent than that of the offered QoS profile.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Connections are only made if the every policy of the requested QoS profile is not more stringent than that of the offered QoS profile.
Connections are only made if every policy of the requested QoS profile is not more stringent than that of the offered QoS profile.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

QoS profile compatibility is determined based on a “Request vs Offered” model.
Subscriptions *request* a QoS profile that is the “minimum quality” that it is willing to accept, and publishers *offer* a QoS profile that is the “maximum quality” that it is able to provide.

Connections are only made if the every policy of the requested QoS profile is not more stringent than that of the offered QoS profile.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if there's any benefit to saying "not more stringent" as opposed to "less stringent" -- if there is, ignore this

Suggested change
Connections are only made if the every policy of the requested QoS profile is not more stringent than that of the offered QoS profile.
Connections are only made if every policy of the requested QoS profile is less stringent than that of the offered QoS profile.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I think it's because we need to convey "less or same", so "not more" might be more ergonomic than something like "less or similarly" when there is the "than" preposition that follows "stringent".

@@ -138,33 +187,161 @@ The following tables show the compatibility of the different policy settings and
- Yes
- Transient local

*Compatibility of deadline QoS policies:*
Copy link
Contributor

@maryaB-osr maryaB-osr May 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There might be some value in adding a simple key, maybe just for the tables that use symbols or maybe for the entire table section. Something like

x, y = arbitrary custom value
● = undefined (?)

Copy link
Member Author

@mm318 mm318 May 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added a short description of x and y and removed appearances of (which were meant to represent N/A).


In order for a connection to be made, all of the policies that affect compatibility must be compatible.
That is, even if a publisher-subscriber pair has compatible reliability QoS profiles, if they have incompatible durability QoS profiles a connection will not be made, and vice-versa.
That is for example, even if a requested and offered QoS profile pair has compatible reliability QoS policies, but they have incompatible durability QoS policies, a connection will still not be made.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
That is for example, even if a requested and offered QoS profile pair has compatible reliability QoS policies, but they have incompatible durability QoS policies, a connection will still not be made.
For example, even if a requested and offered QoS profile pair has compatible reliability QoS policies, but they have incompatible durability QoS policies, a connection will still not be made.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

That is for example, even if a requested and offered QoS profile pair has compatible reliability QoS policies, but they have incompatible durability QoS policies, a connection will still not be made.

When connections are not made, no messages will be passed between the publisher and subscription.
There are mechanisms to detect this situation, which will be covered in a later section below.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
There are mechanisms to detect this situation, which will be covered in a later section below.
There are mechanisms to detect this situation, which will be covered in a later section.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Comparison to ROS 1
^^^^^^^^^^^^^^^^^^^

Historically in ROS 1, any publisher and subscriber with the same message type on the same topic would be connected.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might be misunderstanding the context but it seems like you changed every other "subscriber" to "subscription"; maybe you missed this one?

Suggested change
Historically in ROS 1, any publisher and subscriber with the same message type on the same topic would be connected.
Historically in ROS 1, any publisher and subscription with the same message type on the same topic would be connected.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was trying to use "subscriber" when referencing ROS 1, but "subscription" when referring to ROS 2.

----------

Some QoS policies have possible events related to them.
Developers may provide each publisher and subscription callback functions that are triggered by these QoS events and handle them in a way they see fit, similar to how messages received on a topic are handled.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Developers may provide each publisher and subscription callback functions that are triggered by these QoS events and handle them in a way they see fit, similar to how messages received on a topic are handled.
Developers may provide each publisher and subscription with callback functions that are triggered by these QoS events and handle them in a way they see fit, similar to how messages received on a topic are handled.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@maryaB-osr
Copy link
Contributor

Thanks @mm318!

@emersonknapp, would you say this fixes #280? If so, could you add it to the description @mm318 so it closes once this is merged?

@mm318
Copy link
Member Author

mm318 commented May 8, 2020

@emersonknapp, would you say this fixes #280? If so, could you add it to the description @mm318 so it closes once this is merged?

This should definitely fix #280. I'll update the description.

Signed-off-by: Miaofei <miaofei@amazon.com>
Copy link
Contributor

@emersonknapp emersonknapp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good to me :)

Copy link
Contributor

@maryaB-osr maryaB-osr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

mm318 added 2 commits May 11, 2020 14:22
Signed-off-by: Miaofei <miaofei@amazon.com>
Signed-off-by: Miaofei <miaofei@amazon.com>
@mm318
Copy link
Member Author

mm318 commented May 11, 2020

I've updated the documentation to remove mentions of "QoS profile of the connection", because it gives the impression that a publisher matching with a subscription could change its behavior, which would not be accurate.

I've also removed mentions of "manual by node", because it is being removed with ros2/rmw#227.

@mm318
Copy link
Member Author

mm318 commented May 11, 2020

Hi @maryaB-osr, I am happy with the way the documentation is now. Can you see if it can be merged? Thanks!

@maryaB-osr
Copy link
Contributor

Thanks for doing this @mm318!

@maryaB-osr maryaB-osr merged commit c0f0385 into ros2:master May 11, 2020
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.

Document Liveliness, Deadline, and Lifespan on "About Quality of Service Settings" page
3 participants