From 3b04d8e68744b1127d27b8d601624e91359bacaa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Poderoso?= <120394830+JesusPoderoso@users.noreply.github.com> Date: Thu, 4 Apr 2024 12:40:32 +0200 Subject: [PATCH] Force unlimited ResourceLimits if lower or equal to zero (#4617) * Refs #20638: Force unlimited ResourceLimits if lower or equal to zero Signed-off-by: JesusPoderoso * Refs #20722: Apply rev suggestions Signed-off-by: JesusPoderoso * Refs #20722: Include check in previous tests Signed-off-by: JesusPoderoso * Refs #20722: Fix test typo Signed-off-by: JesusPoderoso --------- Signed-off-by: JesusPoderoso (cherry picked from commit 31b8ad145c132aba8440ca9822e759ecdd8b54fe) --- include/fastdds/dds/core/policy/QosPolicies.hpp | 6 +++--- src/cpp/fastdds/publisher/DataWriterHistory.cpp | 9 +++++++-- .../subscriber/history/DataReaderHistory.cpp | 6 +++--- .../publisher/PublisherHistory.cpp | 9 +++++++-- .../subscriber/SubscriberHistory.cpp | 6 +++--- .../fastdds/publisher/DataWriterHistory.hpp | 9 +++++++-- .../fastrtps/publisher/PublisherHistory.h | 9 +++++++-- .../performance/throughput/ThroughputPublisher.cpp | 2 +- .../throughput/ThroughputSubscriber.cpp | 2 +- test/unittest/dds/publisher/DataWriterTests.cpp | 14 ++++++++++++-- test/unittest/dds/subscriber/DataReaderTests.cpp | 14 ++++++++++++-- test/unittest/dds/topic/TopicTests.cpp | 14 ++++++++++++-- 12 files changed, 75 insertions(+), 25 deletions(-) diff --git a/include/fastdds/dds/core/policy/QosPolicies.hpp b/include/fastdds/dds/core/policy/QosPolicies.hpp index adc30c0c491..3b0bc1569ed 100644 --- a/include/fastdds/dds/core/policy/QosPolicies.hpp +++ b/include/fastdds/dds/core/policy/QosPolicies.hpp @@ -1727,21 +1727,21 @@ class ResourceLimitsQosPolicy : public Parameter_t, public QosPolicy * @brief Specifies the maximum number of data-samples the DataWriter (or DataReader) can manage across all the * instances associated with it. Represents the maximum samples the middleware can store for any one DataWriter * (or DataReader).
- * Value 0 means infinite resources. By default, 5000. + * Value less or equal to 0 means infinite resources. By default, 5000. * * @warning It is inconsistent if `max_samples < (max_instances * max_samples_per_instance)`. */ int32_t max_samples; /** * @brief Represents the maximum number of instances DataWriter (or DataReader) can manage.
- * Value 0 means infinite resources. By default, 10. + * Value less or equal to 0 means infinite resources. By default, 10. * * @warning It is inconsistent if `(max_instances * max_samples_per_instance) > max_samples`. */ int32_t max_instances; /** * @brief Represents the maximum number of samples of any one instance a DataWriter(or DataReader) can manage.
- * Value 0 means infinite resources. By default, 400. + * Value less or equal to 0 means infinite resources. By default, 400. * * @warning It is inconsistent if `(max_instances * max_samples_per_instance) > max_samples`. */ diff --git a/src/cpp/fastdds/publisher/DataWriterHistory.cpp b/src/cpp/fastdds/publisher/DataWriterHistory.cpp index 07a6662f37c..a5e2f4a9811 100644 --- a/src/cpp/fastdds/publisher/DataWriterHistory.cpp +++ b/src/cpp/fastdds/publisher/DataWriterHistory.cpp @@ -67,12 +67,17 @@ DataWriterHistory::DataWriterHistory( , topic_att_(topic_att) , unacknowledged_sample_removed_functor_(unack_sample_remove_functor) { - if (resource_limited_qos_.max_instances == 0) + if (resource_limited_qos_.max_samples <= 0) + { + resource_limited_qos_.max_samples = std::numeric_limits::max(); + } + + if (resource_limited_qos_.max_instances <= 0) { resource_limited_qos_.max_instances = std::numeric_limits::max(); } - if (resource_limited_qos_.max_samples_per_instance == 0) + if (resource_limited_qos_.max_samples_per_instance <= 0) { resource_limited_qos_.max_samples_per_instance = std::numeric_limits::max(); } diff --git a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp index 729a8b86f31..c495681a32e 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp @@ -70,17 +70,17 @@ DataReaderHistory::DataReaderHistory( , type_(type.get()) , get_key_object_(nullptr) { - if (resource_limited_qos_.max_samples == 0) + if (resource_limited_qos_.max_samples <= 0) { resource_limited_qos_.max_samples = std::numeric_limits::max(); } - if (resource_limited_qos_.max_instances == 0) + if (resource_limited_qos_.max_instances <= 0) { resource_limited_qos_.max_instances = std::numeric_limits::max(); } - if (resource_limited_qos_.max_samples_per_instance == 0) + if (resource_limited_qos_.max_samples_per_instance <= 0) { resource_limited_qos_.max_samples_per_instance = std::numeric_limits::max(); } diff --git a/src/cpp/fastrtps_deprecated/publisher/PublisherHistory.cpp b/src/cpp/fastrtps_deprecated/publisher/PublisherHistory.cpp index f7fe88f2612..59b27168163 100644 --- a/src/cpp/fastrtps_deprecated/publisher/PublisherHistory.cpp +++ b/src/cpp/fastrtps_deprecated/publisher/PublisherHistory.cpp @@ -66,12 +66,17 @@ PublisherHistory::PublisherHistory( , resource_limited_qos_(topic_att.resourceLimitsQos) , topic_att_(topic_att) { - if (resource_limited_qos_.max_instances == 0) + if (resource_limited_qos_.max_samples <= 0) + { + resource_limited_qos_.max_samples = std::numeric_limits::max(); + } + + if (resource_limited_qos_.max_instances <= 0) { resource_limited_qos_.max_instances = std::numeric_limits::max(); } - if (resource_limited_qos_.max_samples_per_instance == 0) + if (resource_limited_qos_.max_samples_per_instance <= 0) { resource_limited_qos_.max_samples_per_instance = std::numeric_limits::max(); } diff --git a/src/cpp/fastrtps_deprecated/subscriber/SubscriberHistory.cpp b/src/cpp/fastrtps_deprecated/subscriber/SubscriberHistory.cpp index 573931a0e6e..6cc37ed0269 100644 --- a/src/cpp/fastrtps_deprecated/subscriber/SubscriberHistory.cpp +++ b/src/cpp/fastrtps_deprecated/subscriber/SubscriberHistory.cpp @@ -96,17 +96,17 @@ SubscriberHistory::SubscriberHistory( get_key_object_ = type_->createData(); } - if (resource_limited_qos_.max_samples == 0) + if (resource_limited_qos_.max_samples <= 0) { resource_limited_qos_.max_samples = std::numeric_limits::max(); } - if (resource_limited_qos_.max_instances == 0) + if (resource_limited_qos_.max_instances <= 0) { resource_limited_qos_.max_instances = std::numeric_limits::max(); } - if (resource_limited_qos_.max_samples_per_instance == 0) + if (resource_limited_qos_.max_samples_per_instance <= 0) { resource_limited_qos_.max_samples_per_instance = std::numeric_limits::max(); } diff --git a/test/mock/rtps/PublisherHistory/fastdds/publisher/DataWriterHistory.hpp b/test/mock/rtps/PublisherHistory/fastdds/publisher/DataWriterHistory.hpp index 1a0613dbfb3..f8de9591e5b 100644 --- a/test/mock/rtps/PublisherHistory/fastdds/publisher/DataWriterHistory.hpp +++ b/test/mock/rtps/PublisherHistory/fastdds/publisher/DataWriterHistory.hpp @@ -79,12 +79,17 @@ class DataWriterHistory : public WriterHistory , topic_att_(topic_att) , unacknowledged_sample_removed_functor_(unack_sample_remove_functor) { - if (resource_limited_qos_.max_instances == 0) + if (resource_limited_qos_.max_samples <= 0) + { + resource_limited_qos_.max_samples = std::numeric_limits::max(); + } + + if (resource_limited_qos_.max_instances <= 0) { resource_limited_qos_.max_instances = std::numeric_limits::max(); } - if (resource_limited_qos_.max_samples_per_instance == 0) + if (resource_limited_qos_.max_samples_per_instance <= 0) { resource_limited_qos_.max_samples_per_instance = std::numeric_limits::max(); } diff --git a/test/mock/rtps/PublisherHistory/fastrtps/publisher/PublisherHistory.h b/test/mock/rtps/PublisherHistory/fastrtps/publisher/PublisherHistory.h index dec5df5ed8d..12655721d96 100644 --- a/test/mock/rtps/PublisherHistory/fastrtps/publisher/PublisherHistory.h +++ b/test/mock/rtps/PublisherHistory/fastrtps/publisher/PublisherHistory.h @@ -74,12 +74,17 @@ class PublisherHistory : public WriterHistory , resource_limited_qos_(topic_att.resourceLimitsQos) , topic_att_(topic_att) { - if (resource_limited_qos_.max_instances == 0) + if (resource_limited_qos_.max_samples <= 0) + { + resource_limited_qos_.max_samples = std::numeric_limits::max(); + } + + if (resource_limited_qos_.max_instances <= 0) { resource_limited_qos_.max_instances = std::numeric_limits::max(); } - if (resource_limited_qos_.max_samples_per_instance == 0) + if (resource_limited_qos_.max_samples_per_instance <= 0) { resource_limited_qos_.max_samples_per_instance = std::numeric_limits::max(); } diff --git a/test/performance/throughput/ThroughputPublisher.cpp b/test/performance/throughput/ThroughputPublisher.cpp index d30b64f5f3e..43b4718f409 100644 --- a/test/performance/throughput/ThroughputPublisher.cpp +++ b/test/performance/throughput/ThroughputPublisher.cpp @@ -526,7 +526,7 @@ void ThroughputPublisher::run( else { // Ensure that the max samples is at least the demand - if (dw_qos.resource_limits().max_samples < 0 || + if (dw_qos.resource_limits().max_samples <= 0 || static_cast(dw_qos.resource_limits().max_samples) < max_demand) { EPROSIMA_LOG_WARNING(THROUGHPUTPUBLISHER, "Setting resource limit max samples to " << max_demand); diff --git a/test/performance/throughput/ThroughputSubscriber.cpp b/test/performance/throughput/ThroughputSubscriber.cpp index 2a790d68aaa..3f80842eb03 100644 --- a/test/performance/throughput/ThroughputSubscriber.cpp +++ b/test/performance/throughput/ThroughputSubscriber.cpp @@ -583,7 +583,7 @@ int ThroughputSubscriber::process_message() else { // Ensure that the max samples is at least the demand - if (dr_qos.resource_limits().max_samples < 0 || + if (dr_qos.resource_limits().max_samples <= 0 || static_cast(dr_qos.resource_limits().max_samples) < max_demand) { EPROSIMA_LOG_WARNING(THROUGHPUTSUBSCRIBER, diff --git a/test/unittest/dds/publisher/DataWriterTests.cpp b/test/unittest/dds/publisher/DataWriterTests.cpp index 6d49620d227..20ca0f7de6b 100644 --- a/test/unittest/dds/publisher/DataWriterTests.cpp +++ b/test/unittest/dds/publisher/DataWriterTests.cpp @@ -1751,7 +1751,10 @@ TEST(DataWriterTests, InstancePolicyAllocationConsistencyNotKeyed) // Below an ampliation of the last comprobation, for which it is proved the case of < 0 (-1), // which also means infinite value, and does not make any change. + // Updated to check negative values (Redmine ticket #20722) + qos.resource_limits().max_samples = -1; qos.resource_limits().max_instances = -1; + qos.resource_limits().max_samples_per_instance = -1; DataWriter* data_writer2 = publisher->create_datawriter(topic, qos); ASSERT_NE(data_writer2, nullptr); @@ -1798,7 +1801,10 @@ TEST(DataWriterTests, InstancePolicyAllocationConsistencyNotKeyed) // Below an ampliation of the last comprobation, for which it is proved the case of < 0 (-1), // which also means infinite value. // By not using instances, instance allocation consistency is not checked. + // Updated to check negative values (Redmine ticket #20722) + qos2.resource_limits().max_samples = -1; qos2.resource_limits().max_instances = -1; + qos2.resource_limits().max_samples_per_instance = -1; ASSERT_EQ(ReturnCode_t::RETCODE_OK, default_data_writer1->set_qos(qos2)); @@ -1869,8 +1875,10 @@ TEST(DataWriterTests, InstancePolicyAllocationConsistencyKeyed) // Below an ampliation of the last comprobation, for which it is proved the case of < 0 (-1), // which also means infinite value. - qos.resource_limits().max_samples = 0; + // Updated to check negative values (Redmine ticket #20722) + qos.resource_limits().max_samples = -1; qos.resource_limits().max_instances = -1; + qos.resource_limits().max_samples_per_instance = -1; DataWriter* data_writer2 = publisher->create_datawriter(topic, qos); ASSERT_NE(data_writer2, nullptr); @@ -1922,8 +1930,10 @@ TEST(DataWriterTests, InstancePolicyAllocationConsistencyKeyed) // Below an ampliation of the last comprobation, for which it is proved the case of < 0 (-1), // which also means infinite value. - qos2.resource_limits().max_samples = 0; + // Updated to check negative values (Redmine ticket #20722) + qos2.resource_limits().max_samples = -1; qos2.resource_limits().max_instances = -1; + qos2.resource_limits().max_samples_per_instance = -1; ASSERT_EQ(ReturnCode_t::RETCODE_OK, default_data_writer1->set_qos(qos2)); diff --git a/test/unittest/dds/subscriber/DataReaderTests.cpp b/test/unittest/dds/subscriber/DataReaderTests.cpp index abbc7a82762..2f2a8441aa8 100644 --- a/test/unittest/dds/subscriber/DataReaderTests.cpp +++ b/test/unittest/dds/subscriber/DataReaderTests.cpp @@ -3265,7 +3265,10 @@ TEST_F(DataReaderTests, InstancePolicyAllocationConsistencyNotKeyed) // Below an ampliation of the last comprobation, for which it is proved the case of < 0 (-1), // which also means infinite value, and does not make any change. + // Updated to check negative values (Redmine ticket #20722) + qos.resource_limits().max_samples = -1; qos.resource_limits().max_instances = -1; + qos.resource_limits().max_samples_per_instance = -1; DataReader* data_reader2 = subscriber->create_datareader(topic, qos); ASSERT_NE(data_reader2, nullptr); @@ -3318,7 +3321,10 @@ TEST_F(DataReaderTests, InstancePolicyAllocationConsistencyNotKeyed) // Below an ampliation of the last comprobation, for which it is proved the case of < 0 (-1), // which also means infinite value. // By not using instances, instance allocation consistency is not checked. + // Updated to check negative values (Redmine ticket #20722) + qos2.resource_limits().max_samples = -1; qos2.resource_limits().max_instances = -1; + qos2.resource_limits().max_samples_per_instance = -1; ASSERT_EQ(ReturnCode_t::RETCODE_OK, default_data_reader2->set_qos(qos2)); @@ -3389,8 +3395,10 @@ TEST_F(DataReaderTests, InstancePolicyAllocationConsistencyKeyed) // Below an ampliation of the last comprobation, for which it is proved the case of < 0 (-1), // which also means infinite value. - qos.resource_limits().max_samples = 0; + // Updated to check negative values (Redmine ticket #20722) + qos.resource_limits().max_samples = -1; qos.resource_limits().max_instances = -1; + qos.resource_limits().max_samples_per_instance = -1; DataReader* data_reader2 = subscriber->create_datareader(topic, qos); ASSERT_NE(data_reader2, nullptr); @@ -3448,8 +3456,10 @@ TEST_F(DataReaderTests, InstancePolicyAllocationConsistencyKeyed) // Below an ampliation of the last comprobation, for which it is proved the case of < 0 (-1), // which also means infinite value. - qos2.resource_limits().max_samples = 0; + // Updated to check negative values (Redmine ticket #20722) + qos2.resource_limits().max_samples = -1; qos2.resource_limits().max_instances = -1; + qos2.resource_limits().max_samples_per_instance = -1; ASSERT_EQ(ReturnCode_t::RETCODE_OK, default_data_reader2->set_qos(qos2)); diff --git a/test/unittest/dds/topic/TopicTests.cpp b/test/unittest/dds/topic/TopicTests.cpp index a5bb4c4aa9f..0db029892c0 100644 --- a/test/unittest/dds/topic/TopicTests.cpp +++ b/test/unittest/dds/topic/TopicTests.cpp @@ -257,7 +257,10 @@ TEST(TopicTests, InstancePolicyAllocationConsistencyNotKeyed) // Below an ampliation of the last comprobation, for which it is proved the case of < 0 (-1), // which also means infinite value, and does not make any change. + // Updated to check negative values (Redmine ticket #20722) + qos.resource_limits().max_samples = -1; qos.resource_limits().max_instances = -1; + qos.resource_limits().max_samples_per_instance = -1; Topic* topic2 = participant->create_topic("footopic2", type.get_type_name(), qos); ASSERT_NE(topic2, nullptr); @@ -304,7 +307,10 @@ TEST(TopicTests, InstancePolicyAllocationConsistencyNotKeyed) // Below an ampliation of the last comprobation, for which it is proved the case of < 0 (-1), // which also means infinite value. // By not using instances, instance allocation consistency is not checked. + // Updated to check negative values (Redmine ticket #20722) + qos2.resource_limits().max_samples = -1; qos2.resource_limits().max_instances = -1; + qos2.resource_limits().max_samples_per_instance = -1; ASSERT_EQ(ReturnCode_t::RETCODE_OK, default_topic1->set_qos(qos2)); @@ -373,8 +379,10 @@ TEST(TopicTests, InstancePolicyAllocationConsistencyKeyed) // Below an ampliation of the last comprobation, for which it is proved the case of < 0 (-1), // which also means infinite value. - qos.resource_limits().max_samples = 0; + // Updated to check negative values (Redmine ticket #20722) + qos.resource_limits().max_samples = -1; qos.resource_limits().max_instances = -1; + qos.resource_limits().max_samples_per_instance = -1; Topic* topic2 = participant->create_topic("footopic2", type.get_type_name(), qos); ASSERT_NE(topic2, nullptr); @@ -426,8 +434,10 @@ TEST(TopicTests, InstancePolicyAllocationConsistencyKeyed) // Below an ampliation of the last comprobation, for which it is proved the case of < 0 (-1), // which also means infinite value. - qos2.resource_limits().max_samples = 0; + // Updated to check negative values (Redmine ticket #20722) + qos2.resource_limits().max_samples = -1; qos2.resource_limits().max_instances = -1; + qos2.resource_limits().max_samples_per_instance = -1; ASSERT_EQ(ReturnCode_t::RETCODE_OK, default_topic1->set_qos(qos2));