diff --git a/src/cpp/fastdds/publisher/DataWriterImpl.cpp b/src/cpp/fastdds/publisher/DataWriterImpl.cpp index 8beb5ecaf16..5b95809ce15 100644 --- a/src/cpp/fastdds/publisher/DataWriterImpl.cpp +++ b/src/cpp/fastdds/publisher/DataWriterImpl.cpp @@ -1826,9 +1826,11 @@ ReturnCode_t DataWriterImpl::check_qos( 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; + EPROSIMA_LOG_WARNING(RTPS_QOS_CHECK, + "HISTORY DEPTH '" << qos.history().depth << + "' is inconsistent with max_samples_per_instance: '" << qos.resource_limits().max_samples_per_instance << + "'. Consistency rule: depth <= max_samples_per_instance." << + " Effectively using max_samples_per_instance as depth."); } return ReturnCode_t::RETCODE_OK; } diff --git a/src/cpp/fastdds/subscriber/DataReaderImpl.cpp b/src/cpp/fastdds/subscriber/DataReaderImpl.cpp index aff75a80f3b..f4aeb8781f1 100644 --- a/src/cpp/fastdds/subscriber/DataReaderImpl.cpp +++ b/src/cpp/fastdds/subscriber/DataReaderImpl.cpp @@ -1466,9 +1466,11 @@ ReturnCode_t DataReaderImpl::check_qos( 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; + EPROSIMA_LOG_WARNING(RTPS_QOS_CHECK, + "HISTORY DEPTH '" << qos.history().depth << + "' is inconsistent with max_samples_per_instance: '" << qos.resource_limits().max_samples_per_instance << + "'. Consistency rule: depth <= max_samples_per_instance." << + " Effectively using max_samples_per_instance as depth."); } return ReturnCode_t::RETCODE_OK; } diff --git a/test/blackbox/common/DDSBlackboxTestsDataReader.cpp b/test/blackbox/common/DDSBlackboxTestsDataReader.cpp index 46778700c49..d2bb301541b 100644 --- a/test/blackbox/common/DDSBlackboxTestsDataReader.cpp +++ b/test/blackbox/common/DDSBlackboxTestsDataReader.cpp @@ -297,6 +297,17 @@ TEST(DDSDataReader, ConsistentReliabilityWhenIntraprocess) xmlparser::XMLProfileManager::library_settings(library_settings); } +/** + * This is a regression test for issue https://eprosima.easyredmine.com/issues/20504. + * It checks that a DataReader be created with default Qos and a large history depth. + */ +TEST(DDSDataReader, default_qos_large_history_depth) +{ + PubSubReader reader(TEST_TOPIC_NAME); + reader.history_depth(1000).init(); + ASSERT_TRUE(reader.isInitialized()); +} + #ifdef INSTANTIATE_TEST_SUITE_P #define GTEST_INSTANTIATE_TEST_MACRO(x, y, z, w) INSTANTIATE_TEST_SUITE_P(x, y, z, w) #else diff --git a/test/blackbox/common/DDSBlackboxTestsDataWriter.cpp b/test/blackbox/common/DDSBlackboxTestsDataWriter.cpp index 4049c25e988..b1c55d155a6 100644 --- a/test/blackbox/common/DDSBlackboxTestsDataWriter.cpp +++ b/test/blackbox/common/DDSBlackboxTestsDataWriter.cpp @@ -421,6 +421,17 @@ TEST(DDSDataWriter, HeartbeatWhileDestruction) } } +/** + * This is a regression test for issue https://eprosima.easyredmine.com/issues/20504. + * It checks that a DataWriter be created with default Qos and a large history depth. + */ +TEST(DDSDataWriter, default_qos_large_history_depth) +{ + PubSubWriter writer(TEST_TOPIC_NAME); + writer.history_depth(1000).init(); + ASSERT_TRUE(writer.isInitialized()); +} + #ifdef INSTANTIATE_TEST_SUITE_P #define GTEST_INSTANTIATE_TEST_MACRO(x, y, z, w) INSTANTIATE_TEST_SUITE_P(x, y, z, w) #else diff --git a/test/unittest/dds/publisher/DataWriterTests.cpp b/test/unittest/dds/publisher/DataWriterTests.cpp index 8f2560ed757..6d49620d227 100644 --- a/test/unittest/dds/publisher/DataWriterTests.cpp +++ b/test/unittest/dds/publisher/DataWriterTests.cpp @@ -12,7 +12,10 @@ // See the License for the specific language governing permissions and // limitations under the License. +#include #include +#include +#include #include #include @@ -28,6 +31,7 @@ #include #include #include +#include #include #include #include @@ -36,14 +40,13 @@ #include #include #include -#include #include #include #include -#include "../../logging/mock/MockConsumer.h" #include "../../common/CustomPayloadPool.hpp" +#include "../../logging/mock/MockConsumer.h" #include #include @@ -681,8 +684,10 @@ TEST(DataWriterTests, InvalidQos) 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 + // KEEP LAST 2000 but max_samples_per_instance default (400) is inconsistent but right now it only shows a warning + // This test will fail whenever we enforce the consistency between depth and max_samples_per_instance. + qos.history().depth = 2000; + EXPECT_EQ(ReturnCode_t::RETCODE_OK, datawriter->set_qos(qos)); ASSERT_TRUE(publisher->delete_datawriter(datawriter) == ReturnCode_t::RETCODE_OK); ASSERT_TRUE(participant->delete_topic(topic) == ReturnCode_t::RETCODE_OK); @@ -2006,6 +2011,75 @@ TEST(DataWriterTests, CustomPoolCreation) DomainParticipantFactory::get_instance()->delete_participant(participant); } +TEST(DataWriterTests, history_depth_max_samples_per_instance_warning) +{ + + /* Setup log so it may catch the expected warning */ + Log::ClearConsumers(); + MockConsumer* mockConsumer = new MockConsumer("RTPS_QOS_CHECK"); + Log::RegisterConsumer(std::unique_ptr(mockConsumer)); + Log::SetVerbosity(Log::Warning); + + /* Create a participant, topic, and a publisher */ + DomainParticipant* participant = DomainParticipantFactory::get_instance()->create_participant(0, + PARTICIPANT_QOS_DEFAULT); + ASSERT_NE(participant, nullptr); + + TypeSupport type(new TopicDataTypeMock()); + type.register_type(participant); + + Topic* topic = participant->create_topic("footopic", type.get_type_name(), TOPIC_QOS_DEFAULT); + ASSERT_NE(topic, nullptr); + + Publisher* publisher = participant->create_publisher(PUBLISHER_QOS_DEFAULT); + ASSERT_NE(publisher, nullptr); + + /* Create a datawriter with the QoS that should generate a warning */ + DataWriterQos qos; + qos.history().depth = 10; + qos.resource_limits().max_samples_per_instance = 5; + DataWriter* datawriter_1 = publisher->create_datawriter(topic, qos); + ASSERT_NE(datawriter_1, nullptr); + + /* Check that the config generated a warning */ + auto wait_for_log_entries = + [&mockConsumer](const uint32_t amount, const uint32_t retries, const uint32_t wait_ms) -> size_t + { + size_t entries = 0; + for (uint32_t i = 0; i < retries; i++) + { + entries = mockConsumer->ConsumedEntries().size(); + if (entries >= amount) + { + break; + } + std::this_thread::sleep_for(std::chrono::milliseconds(wait_ms)); + } + return entries; + }; + + const size_t expected_entries = 1; + const uint32_t retries = 4; + const uint32_t wait_ms = 25; + ASSERT_EQ(wait_for_log_entries(expected_entries, retries, wait_ms), expected_entries); + + /* Check that the datawriter can send data */ + FooType data; + ASSERT_EQ(ReturnCode_t::RETCODE_OK, datawriter_1->write(&data, HANDLE_NIL)); + + /* Check that a correctly initialized writer does not produce any warning */ + qos.history().depth = 10; + qos.resource_limits().max_samples_per_instance = 10; + DataWriter* datawriter_2 = publisher->create_datawriter(topic, qos); + ASSERT_NE(datawriter_2, nullptr); + ASSERT_EQ(wait_for_log_entries(expected_entries, retries, wait_ms), expected_entries); + + /* Tear down */ + participant->delete_contained_entities(); + DomainParticipantFactory::get_instance()->delete_participant(participant); + Log::KillThread(); +} + } // namespace dds } // namespace fastdds } // namespace eprosima diff --git a/test/unittest/dds/subscriber/DataReaderTests.cpp b/test/unittest/dds/subscriber/DataReaderTests.cpp index 22357e9a535..abbc7a82762 100644 --- a/test/unittest/dds/subscriber/DataReaderTests.cpp +++ b/test/unittest/dds/subscriber/DataReaderTests.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -22,58 +23,50 @@ #include #include +#include + #include #include #include #include - +#include #include #include #include #include #include -#include #include #include #include - -#include #include +#include #include - +#include #include #include #include #include - #include #include -#include -#include #include #include - +#include +#include #include -#include - -#include "FooBoundedType.hpp" -#include "FooBoundedTypeSupport.hpp" - -#include "FooType.hpp" -#include "FooTypeSupport.hpp" - -#include "../../logging/mock/MockConsumer.h" - #include +#include #include #include "../../common/CustomPayloadPool.hpp" +#include "../../logging/mock/MockConsumer.h" #include "fastdds/dds/common/InstanceHandle.hpp" #include "fastdds/dds/core/policy/QosPolicies.hpp" - -#include +#include "FooBoundedType.hpp" +#include "FooBoundedTypeSupport.hpp" +#include "FooType.hpp" +#include "FooTypeSupport.hpp" #if defined(__cplusplus_winrt) #define GET_PID GetCurrentProcessId @@ -701,9 +694,13 @@ TEST_F(DataReaderTests, InvalidQos) 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 + // KEEP LAST 2000 but max_samples_per_instance default (400) is inconsistent but right now it only shows a warning + // In the reader, this returns RETCODE_INMUTABLE_POLICY, because the depth cannot be changed on run time. + // Because of the implementation, we know de consistency is checked before the inmutability, so by checking the + // return against RETCODE_INMUTABLE_POLICY we are testing that the setting are not considered inconsistent yet. + // This test will fail whenever we enforce the consistency between depth and max_samples_per_instance. + qos.history().depth = 2000; + EXPECT_EQ(ReturnCode_t::RETCODE_IMMUTABLE_POLICY, data_reader_->set_qos(qos)); /* Inmutable QoS */ const ReturnCode_t inmutable_code = ReturnCode_t::RETCODE_IMMUTABLE_POLICY; @@ -3542,6 +3539,71 @@ TEST_F(DataReaderTests, CustomPoolCreation) DomainParticipantFactory::get_instance()->delete_participant(participant); } +TEST_F(DataReaderTests, history_depth_max_samples_per_instance_warning) +{ + + /* Setup log so it may catch the expected warning */ + Log::ClearConsumers(); + MockConsumer* mockConsumer = new MockConsumer("RTPS_QOS_CHECK"); + Log::RegisterConsumer(std::unique_ptr(mockConsumer)); + Log::SetVerbosity(Log::Warning); + + /* Create a participant, topic, and a subscriber */ + DomainParticipant* participant = DomainParticipantFactory::get_instance()->create_participant(0, + PARTICIPANT_QOS_DEFAULT); + ASSERT_NE(participant, nullptr); + + TypeSupport type(new FooTypeSupport()); + type.register_type(participant); + + Topic* topic = participant->create_topic("footopic", type.get_type_name(), TOPIC_QOS_DEFAULT); + ASSERT_NE(topic, nullptr); + + Subscriber* subscriber = participant->create_subscriber(SUBSCRIBER_QOS_DEFAULT); + ASSERT_NE(subscriber, nullptr); + + /* Create a datareader with the QoS that should generate a warning */ + DataReaderQos qos; + qos.history().depth = 10; + qos.resource_limits().max_samples_per_instance = 5; + DataReader* datareader_1 = subscriber->create_datareader(topic, qos); + ASSERT_NE(datareader_1, nullptr); + + /* Check that the config generated a warning */ + auto wait_for_log_entries = + [&mockConsumer](const uint32_t amount, const uint32_t retries, const uint32_t wait_ms) -> size_t + { + size_t entries = 0; + for (uint32_t i = 0; i < retries; i++) + { + entries = mockConsumer->ConsumedEntries().size(); + if (entries >= amount) + { + break; + } + std::this_thread::sleep_for(std::chrono::milliseconds(wait_ms)); + } + return entries; + }; + + const size_t expected_entries = 1; + const uint32_t retries = 4; + const uint32_t wait_ms = 25; + ASSERT_EQ(wait_for_log_entries(expected_entries, retries, wait_ms), expected_entries); + + /* Check that a correctly initialized datareader does not produce any warning */ + qos.history().depth = 10; + qos.resource_limits().max_samples_per_instance = 10; + DataReader* datareader_2 = subscriber->create_datareader(topic, qos); + ASSERT_NE(datareader_2, nullptr); + ASSERT_EQ(wait_for_log_entries(expected_entries, retries, wait_ms), expected_entries); + + /* Tear down */ + participant->delete_contained_entities(); + DomainParticipantFactory::get_instance()->delete_participant(participant); + Log::KillThread(); +} + int main( int argc, char** argv)