Pick smallest available participant ID for new paricipants

[sloretz]

Picks the smallest available participant ID.

Description

This picks the smallest available participant ID when creating new participants.
This is necessary for the out of box discovery improvements in ROS 2: ros2/rmw_fastrtps#653

The issue with picking one greater than the maximum participant ID is that it's possible for the ID to increase well beyond the number of alive participants. For example, Participant A is created with 0, and B is created with 1. A is destroyed, and C is created. C takes ID 2 even though 0 is available and there are only two participants. When using unicast discovery this can cause participants to no longer be discoverable after the maximum ID climbs past maxInititialPeersRange. This is currently causing the test_launch_ros tests to fail with the change to unicast discovery by default.

Assuming the change looks ok, would it be possible to backport this to the 2.10 branch (assuming that's the branch that will be used for ROS Iron)?

Contributor Checklist

• Commit messages follow the project guidelines.
• The code follows the style guidelines of this project.
• Tests that thoroughly check the new feature have been added/Regression tests checking the bug and its fix have been added; the added tests pass locally
• Any new/modified methods have been properly documented using Doxygen.
• Changes are ABI compatible.
• Changes are API compatible.
• N/A New feature has been added to the versions.md file (if applicable).
• N/A New feature has been documented/Current behavior is correctly described in the documentation.
• N/A Applicable backports have been included in the description.

Reviewer Checklist

• The PR has a milestone assigned.
• Check contributor checklist is correct.
• Check CI results: changes do not issue any warning.
• Check CI results: failing tests are unrelated with the changes.

[MiguelCompany]

@richiprosima please test this

[MiguelCompany]

@sloretz Reusing a participant ID within the same process is a bad idea, since it will allow for two participants to have the same GUID.

I am developing a different way of achieving the expected behavior, where the ports used do not depend on the participantID, and we always try with the lowest possible port.

[sloretz]

I am developing a different way of achieving the expected behavior, where the ports used do not depend on the participantID, and we always try with the lowest possible port.

@MiguelCompany I thought the ports being calculated from the participant ID was part of the RTPS standard.

https://fast-dds.docs.eprosima.com/en/latest/fastdds/xml_configuration/domainparticipant.html#port-configuration

and

https://www.omg.org/spec/DDSI-RTPS/2.3/PDF

I see in 9.6.1.1 it says

The domainId and participantId identifiers are used to avoid port conflicts among Participants on the same
node. Each Participant on the same node and in the same domain must use a unique participantId. In the case
of multicast, all Participants in the same domain share the same port number, so the participantId identifier is
not used in the port number expression.

It seems like it would be OK to reuse a participant ID as no two participants would have the same ID at the same time. If I understand correctly, wouldn't the same thing happen if participants were in different processes and someone were to stop and restart the last participant on a machine?

If the GUID being reused is a problem, maybe a solution is to change how the participant GUID is generated? It looks like the standard is more flexible about that.

8.2.4.2 The GUIDs of RTPS Participants
Every Participant has GUID <prefix, ENTITYID_PARTICIPANT>, where the constant
ENTITYID_PARTICIPANT is a special value defined by the RTPS protocol. Its actual value depends on the
PSM.

The implementation is free to choose the prefix, as long as every Participant in the Domain has a unique GUID.

[MiguelCompany]

@richiprosima Please test this

[MiguelCompany]

@sloretz On f6f9840c28ed4282a70b95bd32a052ab98a6d46a I added a small change that makes the GUID be different when creating the same participant ID twice.

The idea is to use a counter on the high part of the ID that is used to generate the GUID.

[MiguelCompany]

@richiprosima Please test this

[wjwwood]

lgtm

[MiguelCompany]

@richiprosima Please test this

[MiguelCompany]

@sloretz @wjwwood I rebased this and added a mechanism to keep track of participant ids that are reserved when calling create_participant_guid from here and used later on participant creation here

This should solve the tests that were failing yesterday.

[MiguelCompany]

@EduPonz Would you mind reviewing this?

[EduPonz]

This LGTM. I think the added members (data & functions) would benefit from some hpp documentation. The same goes for some of the not so straight forward code both around the reserved/used pattern (clever way of going around the full locking BTW), and around the get_id_for_prefix function, which shows some nice out-of-the-box thinking 🤯 .

[jepemi]

@Mergifyio backport 2.6.x

[mergify]

backport 2.6.x

✅ Backports have been created

• #4555 Pick smallest available participant ID for new paricipants (backport #3437) has been created for branch 2.6.x but encountered conflicts