Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[21519] Fix issue with exclusive ownership and unordered samples #5182

Merged
merged 5 commits into from
Sep 10, 2024
Merged

[21519] Fix issue with exclusive ownership and unordered samples #5182

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
43f7656
Refs #20866. Regression test.
MiguelCompany Aug 30, 2024
7cd8ed4
Refs #20866. Additional regression test.
MiguelCompany Sep 3, 2024
56c4900
Refs #20866. Fix issue.
MiguelCompany Sep 3, 2024
3548b2f
Refs #20866. Fix unit tests.
MiguelCompany Sep 3, 2024
b886d86
Refs #20866. Refactor test to run several cases.
MiguelCompany Sep 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
163 changes: 70 additions & 93 deletions test/blackbox/common/DDSBlackboxTestsOwnershipQos.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2168,106 +2168,51 @@ TEST_P(OwnershipQos, exclusive_kind_keyed_besteffort_disposing_instance)
/*!
* This is a regression test for redmine issue 20866.
*
* This test checks that a reader with a KEEP_ALL history and an exclusive ownership policy only returns the data
* from the writer with the highest strength.
*/
TEST(OwnershipQos, exclusive_kind_keep_all_reliable)
{
PubSubReader<KeyedHelloWorldPubSubType> reader(TEST_TOPIC_NAME);
PubSubWriter<KeyedHelloWorldPubSubType> low_strength_writer(TEST_TOPIC_NAME);
PubSubWriter<KeyedHelloWorldPubSubType> high_strength_writer(TEST_TOPIC_NAME);

// Prepare data.
std::list<KeyedHelloWorld> generated_data = default_keyedhelloworld_data_generator(20);
auto middle = std::next(generated_data.begin(), 10);
std::list<KeyedHelloWorld> low_strength_data(generated_data.begin(), middle);
std::list<KeyedHelloWorld> high_strength_data(middle, generated_data.end());
auto expected_data = high_strength_data;

// Initialize writers.
low_strength_writer.ownership_strength(3)
.history_kind(eprosima::fastdds::dds::KEEP_ALL_HISTORY_QOS)
.reliability(eprosima::fastdds::dds::RELIABLE_RELIABILITY_QOS)
.init();
ASSERT_TRUE(low_strength_writer.isInitialized());

// High strength writer will use a custom transport to ensure its data is received after the low strength data.
auto test_transport = std::make_shared<test_UDPv4TransportDescriptor>();
std::atomic<bool> drop_messages(false);
test_transport->messages_filter_ = [&drop_messages](eprosima::fastdds::rtps::CDRMessage_t&)
{
return drop_messages.load();
};
high_strength_writer.ownership_strength(4)
.history_kind(eprosima::fastdds::dds::KEEP_ALL_HISTORY_QOS)
.reliability(eprosima::fastdds::dds::RELIABLE_RELIABILITY_QOS)
.disable_builtin_transport()
.add_user_transport_to_pparams(test_transport)
.init();
ASSERT_TRUE(high_strength_writer.isInitialized());

// Initialize reader.
reader.ownership_exclusive()
.history_kind(eprosima::fastdds::dds::KEEP_ALL_HISTORY_QOS)
.reliability(eprosima::fastdds::dds::RELIABLE_RELIABILITY_QOS)
.init();
ASSERT_TRUE(reader.isInitialized());

// Wait for discovery.
low_strength_writer.wait_discovery();
high_strength_writer.wait_discovery();
reader.wait_discovery(std::chrono::seconds::zero(), 2);

// Send high strength data first, so it has the lowest source timestamps, but drop the messages, so they arrive
// later to the reader.
drop_messages.store(true);
high_strength_writer.send(high_strength_data);
EXPECT_TRUE(high_strength_data.empty());

// Send low strength data, so it has the highest source timestamps.
low_strength_writer.send(low_strength_data);
EXPECT_TRUE(low_strength_data.empty());
// Wait for the reader to receive the data.
EXPECT_TRUE(low_strength_writer.waitForAllAcked(std::chrono::seconds(1)));

// Let high strength writer send the data.
drop_messages.store(false);

// Wait for the reader to receive the high strength data.
EXPECT_TRUE(high_strength_writer.waitForAllAcked(std::chrono::seconds(1)));

// Make the reader process the data, expecting only the high strength data.
// The issue was reproduced by the reader complaining about reception of unexpected data.
reader.startReception(expected_data);
reader.block_for_all();
}

/*!
* This is a regression test for redmine issue 20866.
JesusPoderoso marked this conversation as resolved.
Show resolved Hide resolved
* This test checks that a reader keeping a long number of samples and with an exclusive ownership policy only
* returns the data from the writer with the highest strength.
*
* This test checks that a reader with a KEEP_ALL history and an exclusive ownership policy only does not return
* data from the writer with the lowest strength after returning data from the highest one.
* @param use_keep_all_history Whether to use KEEP_ALL history or KEEP_LAST(20).
* @param mixed_data Whether to send data from both writers in an interleaved way.
*/
TEST(OwnershipQos, exclusive_kind_keep_all_reliable_mixed)
static void test_exclusive_kind_big_history(
bool use_keep_all_history,
bool mixed_data)
{
PubSubReader<KeyedHelloWorldPubSubType> reader(TEST_TOPIC_NAME);
PubSubWriter<KeyedHelloWorldPubSubType> low_strength_writer(TEST_TOPIC_NAME);
PubSubWriter<KeyedHelloWorldPubSubType> high_strength_writer(TEST_TOPIC_NAME);

// Configure history QoS.
if (use_keep_all_history)
{
reader.history_kind(eprosima::fastdds::dds::KEEP_ALL_HISTORY_QOS);
low_strength_writer.history_kind(eprosima::fastdds::dds::KEEP_ALL_HISTORY_QOS);
high_strength_writer.history_kind(eprosima::fastdds::dds::KEEP_ALL_HISTORY_QOS);
}
else
{
reader.history_kind(eprosima::fastdds::dds::KEEP_LAST_HISTORY_QOS).history_depth(20);
low_strength_writer.history_kind(eprosima::fastdds::dds::KEEP_LAST_HISTORY_QOS).history_depth(20);
high_strength_writer.history_kind(eprosima::fastdds::dds::KEEP_LAST_HISTORY_QOS).history_depth(20);
}

// Prepare data.
std::list<KeyedHelloWorld> generated_data = default_keyedhelloworld_data_generator(20);
auto middle = std::next(generated_data.begin(), 10);
std::list<KeyedHelloWorld> low_strength_data(generated_data.begin(), middle);
std::list<KeyedHelloWorld> high_strength_data(middle, generated_data.end());
auto expected_data = high_strength_data;
auto it = low_strength_data.begin();
// Expect reception of the first two samples from the low strength writer (one per instance).
expected_data.push_front(*it++);
expected_data.push_front(*it);

if (mixed_data)
{
// Expect reception of the first two samples from the low strength writer (one per instance).
auto it = low_strength_data.begin();
expected_data.push_front(*it++);
expected_data.push_front(*it);
}

// Initialize writers.
low_strength_writer.ownership_strength(3)
.history_kind(eprosima::fastdds::dds::KEEP_ALL_HISTORY_QOS)
.reliability(eprosima::fastdds::dds::RELIABLE_RELIABILITY_QOS)
.init();
ASSERT_TRUE(low_strength_writer.isInitialized());
Expand All @@ -2280,7 +2225,6 @@ TEST(OwnershipQos, exclusive_kind_keep_all_reliable_mixed)
return drop_messages.load();
};
high_strength_writer.ownership_strength(4)
.history_kind(eprosima::fastdds::dds::KEEP_ALL_HISTORY_QOS)
.reliability(eprosima::fastdds::dds::RELIABLE_RELIABILITY_QOS)
.disable_builtin_transport()
.add_user_transport_to_pparams(test_transport)
Expand All @@ -2289,7 +2233,6 @@ TEST(OwnershipQos, exclusive_kind_keep_all_reliable_mixed)

// Initialize reader.
reader.ownership_exclusive()
.history_kind(eprosima::fastdds::dds::KEEP_ALL_HISTORY_QOS)
.reliability(eprosima::fastdds::dds::RELIABLE_RELIABILITY_QOS)
.init();
ASSERT_TRUE(reader.isInitialized());
Expand All @@ -2302,13 +2245,27 @@ TEST(OwnershipQos, exclusive_kind_keep_all_reliable_mixed)
// Drop the messages from the high strength writer, so they arrive later to the reader.
drop_messages.store(true);

// Send one sample from each writer, with low strength data first.
while (!low_strength_data.empty() && !high_strength_data.empty())
if (mixed_data)
{
EXPECT_TRUE(low_strength_writer.send_sample(low_strength_data.front()));
EXPECT_TRUE(high_strength_writer.send_sample(high_strength_data.front()));
low_strength_data.pop_front();
high_strength_data.pop_front();
// Send one sample from each writer, with low strength data first.
while (!low_strength_data.empty() && !high_strength_data.empty())
{
EXPECT_TRUE(low_strength_writer.send_sample(low_strength_data.front()));
EXPECT_TRUE(high_strength_writer.send_sample(high_strength_data.front()));
low_strength_data.pop_front();
high_strength_data.pop_front();
}
}
else
{
// Send high strength data first, so it has the lowest source timestamps, but drop the messages, so they arrive
// later to the reader.
high_strength_writer.send(high_strength_data);
EXPECT_TRUE(high_strength_data.empty());

// Send low strength data, so it has the highest source timestamps.
low_strength_writer.send(low_strength_data);
EXPECT_TRUE(low_strength_data.empty());
}

// Wait for the reader to receive the low strength data.
Expand All @@ -2318,12 +2275,32 @@ TEST(OwnershipQos, exclusive_kind_keep_all_reliable_mixed)
drop_messages.store(false);
EXPECT_TRUE(high_strength_writer.waitForAllAcked(std::chrono::seconds(1)));

// Make the reader process the data, expecting only the high strength data.
// Make the reader process the data, expecting only the required data.
// The issue was reproduced by the reader complaining about reception of unexpected data.
reader.startReception(expected_data);
reader.block_for_all();
}
JesusPoderoso marked this conversation as resolved.
Show resolved Hide resolved

TEST(OwnershipQos, exclusive_kind_keep_all_reliable)
{
test_exclusive_kind_big_history(true, false);
}

TEST(OwnershipQos, exclusive_kind_keep_all_reliable_mixed)
{
test_exclusive_kind_big_history(true, true);
}

TEST(OwnershipQos, exclusive_kind_keep_last_reliable)
{
test_exclusive_kind_big_history(false, false);
}

TEST(OwnershipQos, exclusive_kind_keep_last_reliable_mixed)
{
test_exclusive_kind_big_history(false, true);
}

#ifdef INSTANTIATE_TEST_SUITE_P
#define GTEST_INSTANTIATE_TEST_MACRO(x, y, z, w) INSTANTIATE_TEST_SUITE_P(x, y, z, w)
#else
Expand Down
Loading