Skip to content

Commit

Permalink
Check History QoS inconsistencies (#4375) (#4407)
Browse files Browse the repository at this point in the history 
* 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)

Co-authored-by: Jesús Poderoso <120394830+JesusPoderoso@users.noreply.github.com>
  • Loading branch information
@mergify @JesusPoderoso
mergify[bot] and JesusPoderoso authored Feb 21, 2024
1 parent b262e64 commit 61cd7be
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 7 deletions.
13 changes: 13 additions & 0 deletions src/cpp/fastdds/publisher/DataWriterImpl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1817,6 +1817,19 @@ ReturnCode_t DataWriterImpl::check_qos(
EPROSIMA_LOG_ERROR(RTPS_QOS_CHECK, "DATA_SHARING cannot be used with memory policies other than PREALLOCATED.");
return ReturnCode_t::RETCODE_INCONSISTENT_POLICY;
}
if (qos.history().kind == KEEP_LAST_HISTORY_QOS && qos.history().depth <= 0)
{
EPROSIMA_LOG_ERROR(RTPS_QOS_CHECK, "HISTORY DEPTH must be higher than 0 if HISTORY KIND is KEEP_LAST.");
return ReturnCode_t::RETCODE_INCONSISTENT_POLICY;
}
if (qos.history().kind == KEEP_LAST_HISTORY_QOS && qos.history().depth > 0 &&
qos.resource_limits().max_samples_per_instance > 0 &&
qos.history().depth > qos.resource_limits().max_samples_per_instance)
{
EPROSIMA_LOG_ERROR(RTPS_QOS_CHECK,
"HISTORY DEPTH must be lower or equal to the max_samples_per_instance value.");
return ReturnCode_t::RETCODE_INCONSISTENT_POLICY;
}
return ReturnCode_t::RETCODE_OK;
}

Expand Down
13 changes: 13 additions & 0 deletions src/cpp/fastdds/subscriber/DataReaderImpl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1457,6 +1457,19 @@ ReturnCode_t DataReaderImpl::check_qos(
EPROSIMA_LOG_ERROR(DDS_QOS_CHECK, "unique_network_request cannot be set along specific locators");
return ReturnCode_t::RETCODE_INCONSISTENT_POLICY;
}
if (qos.history().kind == KEEP_LAST_HISTORY_QOS && qos.history().depth <= 0)
{
EPROSIMA_LOG_ERROR(RTPS_QOS_CHECK, "HISTORY DEPTH must be higher than 0 if HISTORY KIND is KEEP_LAST.");
return ReturnCode_t::RETCODE_INCONSISTENT_POLICY;
}
if (qos.history().kind == KEEP_LAST_HISTORY_QOS && qos.history().depth > 0 &&
qos.resource_limits().max_samples_per_instance > 0 &&
qos.history().depth > qos.resource_limits().max_samples_per_instance)
{
EPROSIMA_LOG_ERROR(RTPS_QOS_CHECK,
"HISTORY DEPTH must be lower or equal to the max_samples_per_instance value.");
return ReturnCode_t::RETCODE_INCONSISTENT_POLICY;
}
return ReturnCode_t::RETCODE_OK;
}

Expand Down
19 changes: 14 additions & 5 deletions test/blackbox/common/DDSBlackboxTestsDataWriter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,7 @@ TEST(DDSDataWriter, OfferedDeadlineMissedListener)
* - Only affects TRANSPORT case (UDP or SHM communication, data_sharing and intraprocess disabled)
* - Destruction order matters: writer must be destroyed before reader (otherwise heartbeats would no be sent while
* destroying the writer)
* Edit: this test has been updated to ensure that HistoryQoS and ResourceLimitQoS constraints are met (#20401).
*/
TEST(DDSDataWriter, HeartbeatWhileDestruction)
{
Expand All @@ -392,13 +393,21 @@ TEST(DDSDataWriter, HeartbeatWhileDestruction)
// A high number of samples increases the probability of the data race to occur
size_t n_samples = 1000;

reader.reliability(RELIABLE_RELIABILITY_QOS).durability_kind(TRANSIENT_LOCAL_DURABILITY_QOS).init();
reader.reliability(RELIABLE_RELIABILITY_QOS)
.durability_kind(TRANSIENT_LOCAL_DURABILITY_QOS)
.init();
ASSERT_TRUE(reader.isInitialized());

writer.reliability(RELIABLE_RELIABILITY_QOS).durability_kind(TRANSIENT_LOCAL_DURABILITY_QOS).history_kind(
KEEP_LAST_HISTORY_QOS).history_depth(static_cast<int32_t>(n_samples)).heartbeat_period_seconds(0).
heartbeat_period_nanosec(
20 * 1000).init();
writer.reliability(RELIABLE_RELIABILITY_QOS)
.durability_kind(TRANSIENT_LOCAL_DURABILITY_QOS)
.history_kind(KEEP_LAST_HISTORY_QOS)
.history_depth(static_cast<int32_t>(n_samples))
.resource_limits_max_samples(static_cast<int32_t>(n_samples))
.resource_limits_max_instances(static_cast<int32_t>(1))
.resource_limits_max_samples_per_instance(static_cast<int32_t>(n_samples))
.heartbeat_period_seconds(0)
.heartbeat_period_nanosec(20 * 1000)
.init();
ASSERT_TRUE(writer.isInitialized());

reader.wait_discovery();
Expand Down
5 changes: 3 additions & 2 deletions test/unittest/dds/profiles/test_xml_profile.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -457,16 +457,17 @@
</matchedSubscribersAllocation>
</data_writer>

<!-- This profile has been updated to ensure that HistoryQoS and ResourceLimitQoS constraints are met (#20401) -->
<data_reader profile_name="test_subscriber_profile">
<topic>
<historyQos>
<kind>KEEP_LAST</kind>
<depth>500</depth>
</historyQos>
<resourceLimitsQos>
<max_samples>10</max_samples>
<max_samples>2500</max_samples>
<max_instances>5</max_instances>
<max_samples_per_instance>2</max_samples_per_instance>
<max_samples_per_instance>500</max_samples_per_instance>
<allocated_samples>10</allocated_samples>
</resourceLimitsQos>
</topic>
Expand Down
9 changes: 9 additions & 0 deletions test/unittest/dds/publisher/DataWriterTests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -675,6 +675,15 @@ TEST(DataWriterTests, InvalidQos)
qos.endpoint().history_memory_policy = eprosima::fastrtps::rtps::PREALLOCATED_WITH_REALLOC_MEMORY_MODE;
EXPECT_EQ(ReturnCode_t::RETCODE_OK, datawriter->set_qos(qos));

qos = DATAWRITER_QOS_DEFAULT;
qos.history().kind = KEEP_LAST_HISTORY_QOS;
qos.history().depth = 0;
EXPECT_EQ(inconsistent_code, datawriter->set_qos(qos)); // KEEP LAST 0 is inconsistent
qos.history().depth = 2;
EXPECT_EQ(ReturnCode_t::RETCODE_OK, datawriter->set_qos(qos)); // KEEP LAST 2 is OK
qos.resource_limits().max_samples_per_instance = 1;
EXPECT_EQ(inconsistent_code, datawriter->set_qos(qos)); // KEEP LAST 2 but max_samples_per_instance 1 is inconsistent

ASSERT_TRUE(publisher->delete_datawriter(datawriter) == ReturnCode_t::RETCODE_OK);
ASSERT_TRUE(participant->delete_topic(topic) == ReturnCode_t::RETCODE_OK);
ASSERT_TRUE(participant->delete_publisher(publisher) == ReturnCode_t::RETCODE_OK);
Expand Down
8 changes: 8 additions & 0 deletions test/unittest/dds/subscriber/DataReaderTests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -697,6 +697,14 @@ TEST_F(DataReaderTests, InvalidQos)
qos.properties().properties().emplace_back("fastdds.unique_network_flows", "");
EXPECT_EQ(inconsistent_code, data_reader_->set_qos(qos));

qos = DATAREADER_QOS_DEFAULT;
qos.history().kind = KEEP_LAST_HISTORY_QOS;
qos.history().depth = 0;
EXPECT_EQ(inconsistent_code, data_reader_->set_qos(qos)); // KEEP LAST 0 is inconsistent
qos.history().depth = 2;
qos.resource_limits().max_samples_per_instance = 1;
EXPECT_EQ(inconsistent_code, data_reader_->set_qos(qos)); // KEEP LAST 2 but max_samples_per_instance 1 is inconsistent

/* Inmutable QoS */
const ReturnCode_t inmutable_code = ReturnCode_t::RETCODE_IMMUTABLE_POLICY;

Expand Down

0 comments on commit 61cd7be

Please sign in to comment.