From 31b8ad145c132aba8440ca9822e759ecdd8b54fe 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 --- 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 5ac318e57db..26b5c0b6ac9 100644 --- a/include/fastdds/dds/core/policy/QosPolicies.hpp +++ b/include/fastdds/dds/core/policy/QosPolicies.hpp @@ -1728,21 +1728,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 c8d025a4257..19a2ae66483 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 4796684b483..392c86f4864 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 a327450b34d..a29dd48ac9c 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 ba56678fde8..3e5a668d6cf 100644 --- a/test/performance/throughput/ThroughputPublisher.cpp +++ b/test/performance/throughput/ThroughputPublisher.cpp @@ -527,7 +527,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 70f326bc5c5..b0532bee28c 100644 --- a/test/performance/throughput/ThroughputSubscriber.cpp +++ b/test/performance/throughput/ThroughputSubscriber.cpp @@ -585,7 +585,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 3bd38b5cf91..fe936fd7816 100644 --- a/test/unittest/dds/publisher/DataWriterTests.cpp +++ b/test/unittest/dds/publisher/DataWriterTests.cpp @@ -1820,7 +1820,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); @@ -1867,7 +1870,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)); @@ -1938,8 +1944,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); @@ -1991,8 +1999,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 8b2b7615f4c..7705ebc3efe 100644 --- a/test/unittest/dds/subscriber/DataReaderTests.cpp +++ b/test/unittest/dds/subscriber/DataReaderTests.cpp @@ -3281,7 +3281,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); @@ -3334,7 +3337,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)); @@ -3405,8 +3411,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); @@ -3464,8 +3472,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 7f004619090..bd102150bfe 100644 --- a/test/unittest/dds/topic/TopicTests.cpp +++ b/test/unittest/dds/topic/TopicTests.cpp @@ -261,7 +261,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); @@ -308,7 +311,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)); @@ -377,8 +383,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); @@ -430,8 +438,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));