Fast-DDS icon indicating copy to clipboard operation
Fast-DDS copied to clipboard

Pick smallest available participant ID for new paricipants (backport #3437)

Open mergify[bot] opened this issue 1 year ago • 1 comments

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: https://github.com/ros2/rmw_fastrtps/pull/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

  • [x] Commit messages follow the project guidelines.

  • [x] 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

  • [x] Any new/modified methods have been properly documented using Doxygen.

  • [x] Changes are ABI compatible.

  • [x] 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

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

This is an automatic backport of pull request #3437 done by [Mergify](https://mergify.com).

mergify[bot] avatar Mar 13 '24 11:03 mergify[bot]

Cherry-pick of e46e87550640713c36f71d96e57b37adca749907 has failed:

On branch mergify/bp/2.6.x/pr-3437
Your branch is up to date with 'origin/2.6.x'.

You are currently cherry-picking commit e46e87550.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   src/cpp/rtps/RTPSDomain.cpp
	both modified:   src/cpp/rtps/RTPSDomainImpl.hpp

no changes added to commit (use "git add" and/or "git commit -a")

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

mergify[bot] avatar Mar 13 '24 11:03 mergify[bot]

@richiprosima please test this

Mario-DL avatar Mar 21 '24 09:03 Mario-DL

I am reordering windows and mac ci since there are more test failures than usual

Mario-DL avatar Mar 25 '24 06:03 Mario-DL