[22814] Improve DS routines #5764

cferreiragonz · 2025-04-14T10:26:46Z

Description

This PR aims to improve efficiency of DS routines by reducing the number of messages sent (Data(p), Data(r) and Data(w)). It does so by:

Avoiding re-sending the server's Data(p) to relevant participants every time its own Data(p) is added to the pdp_queue (when a new client is discovered). Now a direct message to the new discovered client is sent instead of adding the server's Data(p) to the queue, which led to multiple sends to all direct clients.
Sending just one Data(p) and Data(r/w) to direct clients by considering a three-state value (unmatched, sent & matched) for DiscoveryParticipantsAckStatus instead of just a boolean. In this way the Discovery DB avoids re-sending repeated data when processing topics.

Related PR:

[22814] Improve DS routines Discovery-Server#115

@Mergifyio backport 3.1.x 2.14.x 2.10.x

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; the added tests pass locally
N/A Any new/modified methods have been properly documented using Doxygen.
N/A Any new configuration API has an equivalent XML API (with the corresponding XSD extension)
Changes are backport compatible: they do NOT break ABI nor change library core behavior.
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.
Applicable backports have been included in the description.

Reviewer Checklist

The PR has a milestone assigned.
The title and description correctly express the PR's purpose.
Check contributor checklist is correct.
If this is a critical bug fix, backports to the critical-only supported branches have been requested.
Check CI results: changes do not issue any warning.
Check CI results: failing tests are unrelated with the changes.

Mario-DL

The approach looks good to me.
Apart from the suggestions below it would be nice if we could add a test. We can make use of test_UDPTransport to detect that the Data(P,W,R) are only sent one time.

src/cpp/rtps/builtin/discovery/database/DiscoveryParticipantsAckStatus.hpp

src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp

src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp

Signed-off-by: cferreiragonz <[email protected]>

cferreiragonz · 2025-04-23T12:30:53Z

In spite of the changes requested, I have added as first two commits the tests to check each scenario solved by this PR.

Signed-off-by: cferreiragonz <[email protected]>

Mario-DL

LGTM,
Please double check the contributor checklist. I think we could backport this improvement

cferreiragonz · 2025-04-24T12:32:21Z

@Mergifyio backport 3.1.x 2.14.x 2.10.x

mergify · 2025-04-24T12:32:28Z

backport 3.1.x 2.14.x 2.10.x

✅ Backports have been created

Details

#5783 [22814] Improve DS routines (backport #5764) has been created for branch 3.1.x
#5784 [22814] Improve DS routines (backport #5764) has been created for branch 2.14.x but encountered conflicts
#5785 [22814] Improve DS routines (backport #5764) has been created for branch 2.10.x but encountered conflicts

* Refs #22814: Data(p) test Signed-off-by: cferreiragonz <[email protected]> * Refs #22814: Data(r/w) test Signed-off-by: cferreiragonz <[email protected]> * Refs #22814: Tristate for ParticipantsAckStatus Signed-off-by: cferreiragonz <[email protected]> * Refs #22814: Send direct messages to new clients Signed-off-by: cferreiragonz <[email protected]> * Refs #22814: Review - Changes Signed-off-by: cferreiragonz <[email protected]> * Refs #22814: Uncrustify Signed-off-by: cferreiragonz <[email protected]> --------- Signed-off-by: cferreiragonz <[email protected]> (cherry picked from commit 0fa7b1e)

* Refs #22814: Data(p) test Signed-off-by: cferreiragonz <[email protected]> * Refs #22814: Data(r/w) test Signed-off-by: cferreiragonz <[email protected]> * Refs #22814: Tristate for ParticipantsAckStatus Signed-off-by: cferreiragonz <[email protected]> * Refs #22814: Send direct messages to new clients Signed-off-by: cferreiragonz <[email protected]> * Refs #22814: Review - Changes Signed-off-by: cferreiragonz <[email protected]> * Refs #22814: Uncrustify Signed-off-by: cferreiragonz <[email protected]> --------- Signed-off-by: cferreiragonz <[email protected]> (cherry picked from commit 0fa7b1e) # Conflicts: # src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp # src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.hpp # src/cpp/rtps/builtin/discovery/database/DiscoveryParticipantsAckStatus.cpp # src/cpp/rtps/builtin/discovery/database/DiscoveryParticipantsAckStatus.hpp # src/cpp/rtps/builtin/discovery/database/DiscoverySharedInfo.hpp # src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp

* Refs #22814: Data(p) test Signed-off-by: cferreiragonz <[email protected]> * Refs #22814: Data(r/w) test Signed-off-by: cferreiragonz <[email protected]> * Refs #22814: Tristate for ParticipantsAckStatus Signed-off-by: cferreiragonz <[email protected]> * Refs #22814: Send direct messages to new clients Signed-off-by: cferreiragonz <[email protected]> * Refs #22814: Review - Changes Signed-off-by: cferreiragonz <[email protected]> * Refs #22814: Uncrustify Signed-off-by: cferreiragonz <[email protected]> --------- Signed-off-by: cferreiragonz <[email protected]> (cherry picked from commit 0fa7b1e) # Conflicts: # src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp # src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.hpp # src/cpp/rtps/builtin/discovery/database/DiscoveryParticipantsAckStatus.cpp # src/cpp/rtps/builtin/discovery/database/DiscoveryParticipantsAckStatus.hpp # src/cpp/rtps/builtin/discovery/database/DiscoverySharedInfo.hpp # src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp # test/blackbox/common/BlackboxTestsDiscovery.cpp

* Refs #22814: Data(p) test Signed-off-by: cferreiragonz <[email protected]> * Refs #22814: Data(r/w) test Signed-off-by: cferreiragonz <[email protected]> * Refs #22814: Tristate for ParticipantsAckStatus Signed-off-by: cferreiragonz <[email protected]> * Refs #22814: Send direct messages to new clients Signed-off-by: cferreiragonz <[email protected]> * Refs #22814: Review - Changes Signed-off-by: cferreiragonz <[email protected]> * Refs #22814: Uncrustify Signed-off-by: cferreiragonz <[email protected]> --------- Signed-off-by: cferreiragonz <[email protected]> (cherry picked from commit 0fa7b1e)

* Improve DS routines (#5764) * Refs #22814: Data(p) test Signed-off-by: cferreiragonz <[email protected]> * Refs #22814: Data(r/w) test Signed-off-by: cferreiragonz <[email protected]> * Refs #22814: Tristate for ParticipantsAckStatus Signed-off-by: cferreiragonz <[email protected]> * Refs #22814: Send direct messages to new clients Signed-off-by: cferreiragonz <[email protected]> * Refs #22814: Review - Changes Signed-off-by: cferreiragonz <[email protected]> * Refs #22814: Uncrustify Signed-off-by: cferreiragonz <[email protected]> --------- Signed-off-by: cferreiragonz <[email protected]> (cherry picked from commit 0fa7b1e) * Fix pdata guid access to 3.1.x Signed-off-by: Mario Dominguez <[email protected]> --------- Signed-off-by: cferreiragonz <[email protected]> Signed-off-by: Mario Dominguez <[email protected]> Co-authored-by: Carlos Ferreira González <[email protected]> Co-authored-by: Mario Dominguez <[email protected]>

* Refs #22814: Data(p) test Signed-off-by: cferreiragonz <[email protected]> * Refs #22814: Data(r/w) test Signed-off-by: cferreiragonz <[email protected]> * Refs #22814: Tristate for ParticipantsAckStatus Signed-off-by: cferreiragonz <[email protected]> * Refs #22814: Send direct messages to new clients Signed-off-by: cferreiragonz <[email protected]> * Refs #22814: Review - Changes Signed-off-by: cferreiragonz <[email protected]> * Refs #22814: Uncrustify Signed-off-by: cferreiragonz <[email protected]> --------- Signed-off-by: cferreiragonz <[email protected]> Signed-off-by: Eugenio Collado <[email protected]>

* Improve DS routines (#5764) * Refs #22814: Data(p) test Signed-off-by: cferreiragonz <[email protected]> * Refs #22814: Data(r/w) test Signed-off-by: cferreiragonz <[email protected]> * Refs #22814: Tristate for ParticipantsAckStatus Signed-off-by: cferreiragonz <[email protected]> * Refs #22814: Send direct messages to new clients Signed-off-by: cferreiragonz <[email protected]> * Refs #22814: Review - Changes Signed-off-by: cferreiragonz <[email protected]> * Refs #22814: Uncrustify Signed-off-by: cferreiragonz <[email protected]> --------- Signed-off-by: cferreiragonz <[email protected]> Signed-off-by: Eugenio Collado <[email protected]> * Fix tests compilation Signed-off-by: Eugenio Collado <[email protected]> * Add GUID prefix and builtin data filter Signed-off-by: cferreiragonz <[email protected]> * Avoid using parametrized test struct Signed-off-by: cferreiragonz <[email protected]> * Move test to DDS suite Signed-off-by: cferreiragonz <[email protected]> --------- Signed-off-by: cferreiragonz <[email protected]> Signed-off-by: Eugenio Collado <[email protected]> Co-authored-by: Carlos Ferreira González <[email protected]> Co-authored-by: Eugenio Collado <[email protected]>

cferreiragonz marked this pull request as ready for review April 14, 2025 11:04

cferreiragonz requested review from richiprosima and removed request for richiprosima April 14, 2025 11:04

github-actions bot added the ci-pending PR which CI is running label Apr 14, 2025

cferreiragonz force-pushed the improve_ds_routines branch 2 times, most recently from f2e1bb7 to f88e09d Compare April 14, 2025 14:52

cferreiragonz requested review from richiprosima and removed request for richiprosima April 14, 2025 14:53

cferreiragonz force-pushed the improve_ds_routines branch 2 times, most recently from 39c54a8 to 35784d6 Compare April 15, 2025 07:05

cferreiragonz requested review from richiprosima and removed request for richiprosima April 15, 2025 07:06

cferreiragonz mentioned this pull request Apr 15, 2025

[22814] Improve DS routines eProsima/Discovery-Server#115

Merged

6 tasks

cferreiragonz changed the title ~~[22814] Improve ds routines~~ [22814] Improve DS routines Apr 15, 2025

cferreiragonz added needs-review PR that is ready to be reviewed and removed ci-pending PR which CI is running labels Apr 15, 2025

Mario-DL suggested changes Apr 21, 2025

View reviewed changes

Mario-DL removed the needs-review PR that is ready to be reviewed label Apr 21, 2025

Refs #22814: Data(p) test

3606373

Signed-off-by: cferreiragonz <[email protected]>

cferreiragonz force-pushed the improve_ds_routines branch from 35784d6 to b6ff33d Compare April 23, 2025 12:24

cferreiragonz added 4 commits April 23, 2025 14:28

Refs #22814: Data(r/w) test

850fcea

Signed-off-by: cferreiragonz <[email protected]>

Refs #22814: Tristate for ParticipantsAckStatus

b7c7757

Signed-off-by: cferreiragonz <[email protected]>

Refs #22814: Send direct messages to new clients

9b19cb6

Signed-off-by: cferreiragonz <[email protected]>

Refs #22814: Review - Changes

f3ab8d3

Signed-off-by: cferreiragonz <[email protected]>

cferreiragonz force-pushed the improve_ds_routines branch from b6ff33d to f3ab8d3 Compare April 23, 2025 12:29

Refs #22814: Uncrustify

07bf34c

Signed-off-by: cferreiragonz <[email protected]>

cferreiragonz requested a review from Mario-DL April 23, 2025 12:32

github-actions bot added the ci-pending PR which CI is running label Apr 23, 2025

rsanchez15 added this to the v3.2.2 milestone Apr 24, 2025

Mario-DL approved these changes Apr 24, 2025

View reviewed changes

cferreiragonz added ready-to-merge Ready to be merged. CI and changes have been reviewed and approved. and removed ci-pending PR which CI is running labels Apr 24, 2025

cferreiragonz merged commit 0fa7b1e into master Apr 25, 2025
17 checks passed

cferreiragonz deleted the improve_ds_routines branch April 25, 2025 06:08

mergify bot mentioned this pull request Apr 25, 2025

[22814] Improve DS routines (backport #5764) #5783

Merged

11 tasks

This was referenced Apr 25, 2025

[22814] Improve DS routines (backport #5764) #5784

Merged

[22814] Improve DS routines (backport #5764) #5785

Closed

cferreiragonz mentioned this pull request Jul 28, 2025

[22814] Fix compatibility with Fast CDR v1 #5958

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[22814] Improve DS routines #5764

[22814] Improve DS routines #5764

Uh oh!

cferreiragonz commented Apr 14, 2025 •

edited

Loading

Uh oh!

Mario-DL left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cferreiragonz commented Apr 23, 2025

Uh oh!

Mario-DL left a comment

Uh oh!

cferreiragonz commented Apr 24, 2025

Uh oh!

mergify bot commented Apr 24, 2025 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

[22814] Improve DS routines #5764

[22814] Improve DS routines #5764

Uh oh!

Conversation

cferreiragonz commented Apr 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Contributor Checklist

Reviewer Checklist

Uh oh!

Mario-DL left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cferreiragonz commented Apr 23, 2025

Uh oh!

Mario-DL left a comment

Choose a reason for hiding this comment

Uh oh!

cferreiragonz commented Apr 24, 2025

Uh oh!

mergify bot commented Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Backports have been created

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

cferreiragonz commented Apr 14, 2025 •

edited

Loading

mergify bot commented Apr 24, 2025 •

edited

Loading