Skip to content

[14091] Fix update remote server locators when enabling statistics#2601

Merged
MiguelCompany merged 2 commits intomasterfrom
hotfix/update-remote-server-locators-with-statistics
Apr 12, 2022
Merged

[14091] Fix update remote server locators when enabling statistics#2601
MiguelCompany merged 2 commits intomasterfrom
hotfix/update-remote-server-locators-with-statistics

Conversation

@JLBuenoLopez
Copy link
Copy Markdown
Contributor

@JLBuenoLopez JLBuenoLopez commented Mar 24, 2022

Signed-off-by: JLBuenoLopez-eProsima joseluisbueno@eprosima.com

Description

If Fast DDS is compile with Statistics (-DFASTDDS_STATISTICS=ON) and in Debug, when updating the remote server locator and assertion in OutputTrafficManager::set_statistics_message_data fails. This is due to the new locator not being added to the collection_ member of this class. New locators are added when calling to the specific transport OpenOutputChannel. This PR ensures that it is called before announcing the new Participant state, preventing the assertion to fail.

Contributor Checklist

  • Commit messages follow the project guidelines.
  • The code follows the style guidelines of this project.
  • N/A Tests that thoroughly check the new feature have been added/Regression tests checking the bug and its fix have been added.

This feature is tested with the tests included in eProsima/Discovery-Server#52. Running these tests with Fast DDS compiled as described before will fail.

  • N/A Any new/modified methods have been properly documented using Doxygen.
  • N/A Fast DDS test suite has been run locally.

Changes does not affect the test suite. RTPSParticipantImpl::update_attributes is called in only a few tests. Changes only affect if there are modified locators (if the feature is being used) and this feature is not tested in Fast DDS suite but in the Discovery Server suite.

  • Changes are ABI compatible.
  • Changes are API compatible.
  • N/A Documentation builds and tests pass locally.
  • 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.

Reviewer Checklist

  • Check contributor checklist is correct.
  • Check CI results: changes do not issue any warning.
  • Check CI results: failing tests are unrelated with the changes.

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
…cators

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
@JLBuenoLopez
Copy link
Copy Markdown
Contributor Author

Ci failures are unrelated

Copy link
Copy Markdown
Member

@MiguelCompany MiguelCompany left a comment

Choose a reason for hiding this comment

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

LGTM. Failures seem unrelated.

@MiguelCompany MiguelCompany merged commit 4624156 into master Apr 12, 2022
@MiguelCompany MiguelCompany deleted the hotfix/update-remote-server-locators-with-statistics branch April 12, 2022 10:27
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.

2 participants