-
Notifications
You must be signed in to change notification settings - Fork 765
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
[20504] Just show warning when inconsistency between depth and max_samples_per_instance #4417
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MiguelCompany
changed the title
Change default resource limits to infinite
[20504] Change default resource limits to infinite
Feb 21, 2024
@clalancette This should fix the regression mentioned in ros2/ros2#1527 Could you please check? |
EduPonz
force-pushed
the
hotfix/default-unlimited-resources
branch
from
February 21, 2024 19:27
19cad85
to
372434d
Compare
EduPonz
previously approved these changes
Feb 21, 2024
EduPonz
force-pushed
the
hotfix/default-unlimited-resources
branch
from
February 22, 2024 10:21
372434d
to
0dbcfe5
Compare
EduPonz
changed the title
[20504] Change default resource limits to infinite
[20504] Just show warning when inconsistency between depth and max_samples_per_instance
Feb 22, 2024
@richiprosima please test this |
JesusPoderoso
previously approved these changes
Feb 22, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with green CI
@Mergifyio backport 2.10.x |
✅ Backports have been created
|
Signed-off-by: EduPonz <eduardoponz@eprosima.com>
Signed-off-by: EduPonz <eduardoponz@eprosima.com>
Signed-off-by: EduPonz <eduardoponz@eprosima.com>
EduPonz
force-pushed
the
hotfix/default-unlimited-resources
branch
from
February 22, 2024 13:49
0dbcfe5
to
dc8d336
Compare
JesusPoderoso
approved these changes
Feb 22, 2024
@richiprosima please test this |
@richiprosima please test windows |
This was referenced Feb 22, 2024
EduPonz
pushed a commit
that referenced
this pull request
Feb 23, 2024
…r_instance (#4417) * Refs #20503. Add regression test Signed-off-by: EduPonz <eduardoponz@eprosima.com> * Refs #20503. Show warning when depth > max_samples_per_instance Signed-off-by: EduPonz <eduardoponz@eprosima.com> * Refs #20503. Fix InvalidQos tests Signed-off-by: EduPonz <eduardoponz@eprosima.com> --------- Signed-off-by: EduPonz <eduardoponz@eprosima.com> Co-authored-by: EduPonz <eduardoponz@eprosima.com> (cherry picked from commit 12201f4)
EduPonz
pushed a commit
that referenced
this pull request
Feb 23, 2024
…r_instance (#4417) (#4440) * Refs #20503. Add regression test Signed-off-by: EduPonz <eduardoponz@eprosima.com> * Refs #20503. Show warning when depth > max_samples_per_instance Signed-off-by: EduPonz <eduardoponz@eprosima.com> * Refs #20503. Fix InvalidQos tests Signed-off-by: EduPonz <eduardoponz@eprosima.com> --------- Signed-off-by: EduPonz <eduardoponz@eprosima.com> Co-authored-by: EduPonz <eduardoponz@eprosima.com> (cherry picked from commit 12201f4) Co-authored-by: Miguel Company <miguelcompany@eprosima.com>
EduPonz
added a commit
that referenced
this pull request
Feb 24, 2024
…r_instance (#4417) * Refs #20503. Add regression test Signed-off-by: EduPonz <eduardoponz@eprosima.com> * Refs #20503. Show warning when depth > max_samples_per_instance Signed-off-by: EduPonz <eduardoponz@eprosima.com> * Refs #20503. Fix InvalidQos tests Signed-off-by: EduPonz <eduardoponz@eprosima.com> --------- Signed-off-by: EduPonz <eduardoponz@eprosima.com> Co-authored-by: EduPonz <eduardoponz@eprosima.com>
elianalf
pushed a commit
that referenced
this pull request
Mar 5, 2024
…r_instance (#4417) * Refs #20503. Add regression test Signed-off-by: EduPonz <eduardoponz@eprosima.com> * Refs #20503. Show warning when depth > max_samples_per_instance Signed-off-by: EduPonz <eduardoponz@eprosima.com> * Refs #20503. Fix InvalidQos tests Signed-off-by: EduPonz <eduardoponz@eprosima.com> --------- Signed-off-by: EduPonz <eduardoponz@eprosima.com> Co-authored-by: EduPonz <eduardoponz@eprosima.com>
EduPonz
added a commit
that referenced
this pull request
Mar 6, 2024
* Check History QoS inconsistencies (#4375) * Refs #20401: Add regression test Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> * Refs #20401: Check History QoS inconsistencies Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> * Refs #20401: Check also the history depth vs resource limits max_sample_per_instance bounds Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> * Refs #20401: Update HeartbeatWhileDestruction test Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> * Refs #20401: Update unit test DDS profiles XML tests profile Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> --------- Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> (cherry picked from commit 68acb5a) * Fix SecureHelloworldExample (#4416) Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com> * Just show warning when inconsistency between depth and max_samples_per_instance (#4417) * Refs #20503. Add regression test Signed-off-by: EduPonz <eduardoponz@eprosima.com> * Refs #20503. Show warning when depth > max_samples_per_instance Signed-off-by: EduPonz <eduardoponz@eprosima.com> * Refs #20503. Fix InvalidQos tests Signed-off-by: EduPonz <eduardoponz@eprosima.com> --------- Signed-off-by: EduPonz <eduardoponz@eprosima.com> Co-authored-by: EduPonz <eduardoponz@eprosima.com> --------- Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com> Signed-off-by: EduPonz <eduardoponz@eprosima.com> Co-authored-by: Jesús Poderoso <120394830+JesusPoderoso@users.noreply.github.com> Co-authored-by: Mario Domínguez López <116071334+Mario-DL@users.noreply.github.com> Co-authored-by: Miguel Company <miguelcompany@eprosima.com> Co-authored-by: EduPonz <eduardoponz@eprosima.com>
Mario-DL
pushed a commit
that referenced
this pull request
Mar 19, 2024
…r_instance (#4417) * Refs #20503. Add regression test Signed-off-by: EduPonz <eduardoponz@eprosima.com> * Refs #20503. Show warning when depth > max_samples_per_instance Signed-off-by: EduPonz <eduardoponz@eprosima.com> * Refs #20503. Fix InvalidQos tests Signed-off-by: EduPonz <eduardoponz@eprosima.com> --------- Signed-off-by: EduPonz <eduardoponz@eprosima.com> Co-authored-by: EduPonz <eduardoponz@eprosima.com>
EduPonz
added a commit
that referenced
this pull request
Apr 10, 2024
* Check History QoS inconsistencies (#4375) * Refs #20401: Add regression test Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> * Refs #20401: Check History QoS inconsistencies Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> * Refs #20401: Check also the history depth vs resource limits max_sample_per_instance bounds Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> * Refs #20401: Update HeartbeatWhileDestruction test Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> * Refs #20401: Update unit test DDS profiles XML tests profile Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> --------- Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> (cherry picked from commit 68acb5a) # Conflicts: # src/cpp/fastdds/publisher/DataWriterImpl.cpp # src/cpp/fastdds/subscriber/DataReaderImpl.cpp # test/unittest/dds/profiles/test_xml_profiles.xml * Refs #20401: Fix conflicts Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> * Fix SecureHelloworldExample (#4416) Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com> * Just show warning when inconsistency between depth and max_samples_per_instance (#4417) * Refs #20503. Add regression test Signed-off-by: EduPonz <eduardoponz@eprosima.com> * Refs #20503. Show warning when depth > max_samples_per_instance Signed-off-by: EduPonz <eduardoponz@eprosima.com> * Refs #20503. Fix InvalidQos tests Signed-off-by: EduPonz <eduardoponz@eprosima.com> --------- Signed-off-by: EduPonz <eduardoponz@eprosima.com> Co-authored-by: EduPonz <eduardoponz@eprosima.com> * Refs #20401: Fix warning Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> * Refs #20401: Fix segfault in Mac tests Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> --------- Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com> Signed-off-by: EduPonz <eduardoponz@eprosima.com> Co-authored-by: Jesús Poderoso <120394830+JesusPoderoso@users.noreply.github.com> Co-authored-by: JesusPoderoso <jesuspoderoso@eprosima.com> Co-authored-by: Mario Domínguez López <116071334+Mario-DL@users.noreply.github.com> Co-authored-by: Miguel Company <miguelcompany@eprosima.com> Co-authored-by: EduPonz <eduardoponz@eprosima.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This addresses a regression in ROS 2 (ros2/ros2#1527) after #4375 was merged.
Whenever a DataWriter is created taking the default
DataWriterQos
, and changing the history depth to a value greater than 400, the creation of the DataWriter fails. Same happens when creating a DataReader.This PR partially reverts #4375, so the inconsistency is shown as a warnings instead of making the entity creation fail.
@Mergifyio backport 2.10.x
*For the other branches, we'll include this as part of the corresponding backport of #4375)
Contributor Checklist
versions.md
file (if applicable).Reviewer Checklist