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

Improve ownership tests #55

Merged
merged 5 commits into from
Oct 28, 2024

Conversation

MiguelCompany
Copy link
Contributor

The check function for the ownership test loops till MAX_SAMPLES_READ samples have been processed.
Those samples should be from the list of published samples stored for each publisher.

In some cases where VOLATILE durability is used, the subscriber might miss the first published samples, which leads to a deadlock in the test (it continuously insists in continuing the loop here)

This PR introduces a mechanism where the publishers communicate the last sample saved in their multiprocess queue.
The check function can then break the loop if one of those samples has already been processed (i.e. it knows the publisher will not be adding elements to the muliprocess queue).

I checked it here with the binary I have prepared for the last release of Fast DDS, and the latest one for Connext DDS

Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
…m a publisher has been processed.

Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
@angelrti
Copy link
Contributor

Changes look good to me

@angelrti angelrti self-requested a review October 22, 2024 15:17
@MiguelCompany MiguelCompany merged commit 8a23c9b into omg-dds:master Oct 28, 2024
@MiguelCompany MiguelCompany deleted the improve-ownership-volatile branch October 28, 2024 07:24
@MiguelCompany
Copy link
Contributor Author

@angelrti I suppose we should create a new release with this. I created a draft one with the binaries from the previous release, but changing the one for Fast DDS.

I launched a test in my fork here

@angelrti
Copy link
Contributor

@MiguelCompany We can create the release and review the results, however we shouldn't publish these results (actions 2, 3 and 4) till OMG Q1 meeting. We can discuss this approach on the upcoming December meeting.

I will make the release 'official' (no draft) and run these tests here.

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