Skip to content

Commit

Permalink
Force unlimited ResourceLimits if lower or equal to zero (#4617) (#4653)
Browse files Browse the repository at this point in the history
* Refs #20638: Force unlimited ResourceLimits if lower or equal to zero

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>

* Refs #20722: Apply rev suggestions

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>

* Refs #20722: Include check in previous tests

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>

* Refs #20722: Fix test typo

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>

---------

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>
(cherry picked from commit 31b8ad1)

Co-authored-by: Jesús Poderoso <120394830+JesusPoderoso@users.noreply.github.com>
  • Loading branch information
mergify[bot] and JesusPoderoso authored Apr 17, 2024
1 parent 748f85e commit 04b35db
Show file tree
Hide file tree
Showing 12 changed files with 75 additions and 25 deletions.
6 changes: 3 additions & 3 deletions include/fastdds/dds/core/policy/QosPolicies.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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). <br>
* 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. <br>
* 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. <br>
* 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`.
*/
Expand Down
9 changes: 7 additions & 2 deletions src/cpp/fastdds/publisher/DataWriterHistory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<int32_t>::max();
}

if (resource_limited_qos_.max_instances <= 0)
{
resource_limited_qos_.max_instances = std::numeric_limits<int32_t>::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<int32_t>::max();
}
Expand Down
6 changes: 3 additions & 3 deletions src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<int32_t>::max();
}

if (resource_limited_qos_.max_instances == 0)
if (resource_limited_qos_.max_instances <= 0)
{
resource_limited_qos_.max_instances = std::numeric_limits<int32_t>::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<int32_t>::max();
}
Expand Down
9 changes: 7 additions & 2 deletions src/cpp/fastrtps_deprecated/publisher/PublisherHistory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<int32_t>::max();
}

if (resource_limited_qos_.max_instances <= 0)
{
resource_limited_qos_.max_instances = std::numeric_limits<int32_t>::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<int32_t>::max();
}
Expand Down
6 changes: 3 additions & 3 deletions src/cpp/fastrtps_deprecated/subscriber/SubscriberHistory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<int32_t>::max();
}

if (resource_limited_qos_.max_instances == 0)
if (resource_limited_qos_.max_instances <= 0)
{
resource_limited_qos_.max_instances = std::numeric_limits<int32_t>::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<int32_t>::max();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<int32_t>::max();
}

if (resource_limited_qos_.max_instances <= 0)
{
resource_limited_qos_.max_instances = std::numeric_limits<int32_t>::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<int32_t>::max();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<int32_t>::max();
}

if (resource_limited_qos_.max_instances <= 0)
{
resource_limited_qos_.max_instances = std::numeric_limits<int32_t>::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<int32_t>::max();
}
Expand Down
2 changes: 1 addition & 1 deletion test/performance/throughput/ThroughputPublisher.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<uint32_t>(dw_qos.resource_limits().max_samples) < max_demand)
{
EPROSIMA_LOG_WARNING(THROUGHPUTPUBLISHER, "Setting resource limit max samples to " << max_demand);
Expand Down
2 changes: 1 addition & 1 deletion test/performance/throughput/ThroughputSubscriber.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<uint32_t>(dr_qos.resource_limits().max_samples) < max_demand)
{
EPROSIMA_LOG_WARNING(THROUGHPUTSUBSCRIBER,
Expand Down
14 changes: 12 additions & 2 deletions test/unittest/dds/publisher/DataWriterTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1755,7 +1755,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);
Expand Down Expand Up @@ -1802,7 +1805,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));

Expand Down Expand Up @@ -1873,8 +1879,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);
Expand Down Expand Up @@ -1926,8 +1934,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));

Expand Down
14 changes: 12 additions & 2 deletions test/unittest/dds/subscriber/DataReaderTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3269,7 +3269,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);
Expand Down Expand Up @@ -3322,7 +3325,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));

Expand Down Expand Up @@ -3393,8 +3399,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);
Expand Down Expand Up @@ -3452,8 +3460,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));

Expand Down
14 changes: 12 additions & 2 deletions test/unittest/dds/topic/TopicTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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));

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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));

Expand Down

0 comments on commit 04b35db

Please sign in to comment.