From 9d2f2d69dfac6c5f47c2ce61e5a5c395ac1310e0 Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Tue, 31 Aug 2021 13:06:06 +0200 Subject: [PATCH 01/56] Refs 12400. Added DataReaderCacheChange. Signed-off-by: Miguel Company --- .../history/DataReaderCacheChange.hpp | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 src/cpp/fastdds/subscriber/history/DataReaderCacheChange.hpp diff --git a/src/cpp/fastdds/subscriber/history/DataReaderCacheChange.hpp b/src/cpp/fastdds/subscriber/history/DataReaderCacheChange.hpp new file mode 100644 index 00000000000..8ef7a9a2fd8 --- /dev/null +++ b/src/cpp/fastdds/subscriber/history/DataReaderCacheChange.hpp @@ -0,0 +1,47 @@ +// Copyright 2021 Proyectos y Sistemas de Mantenimiento SL (eProsima). +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +/** + * @file DataReaderCacheChange.hpp + */ + +#ifndef _FASTDDS_SUBSCRIBER_HISTORY_DATAREADERCACHECHANGE_HPP_ +#define _FASTDDS_SUBSCRIBER_HISTORY_DATAREADERCACHECHANGE_HPP_ + +#include + +#include + +namespace eprosima { +namespace fastdds { +namespace dds { +namespace detail { + +/// A DataReader cache entry +struct DataReaderCacheChange +{ + //! Low level cache entry + fastrtps::rtps::CacheChange_t* change; + //! Disposed generation of the instance when this entry was added to it + int32_t disposed_generation_count; + //! No-writers generation of the instance when this entry was added to it + int32_t no_writers_generation_count; +}; + +} /* namespace detail */ +} /* namespace dds */ +} /* namespace fastdds */ +} /* namespace eprosima */ + +#endif // _FASTDDS_SUBSCRIBER_HISTORY_DATAREADERCACHECHANGE_HPP_ From 52b6c9f3e787916f57695339dbfb7e399e22f14b Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Wed, 1 Sep 2021 13:39:36 +0200 Subject: [PATCH 02/56] Refs 12400. Added DataReaderInstance. Signed-off-by: Miguel Company --- .../subscriber/history/DataReaderInstance.hpp | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp diff --git a/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp b/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp new file mode 100644 index 00000000000..76d218a163a --- /dev/null +++ b/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp @@ -0,0 +1,61 @@ +// Copyright 2021 Proyectos y Sistemas de Mantenimiento SL (eProsima). +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +/** + * @file DataReaderInstance.hpp + */ + +#ifndef _FASTDDS_SUBSCRIBER_HISTORY_DATAREADERINSTANCE_HPP_ +#define _FASTDDS_SUBSCRIBER_HISTORY_DATAREADERINSTANCE_HPP_ + +#include +#include + +#include +#include + +#include + +#include "DataReaderCacheChange.hpp" + +namespace eprosima { +namespace fastdds { +namespace dds { +namespace detail { + +/// Book-keeping information for an instance +struct DataReaderInstance +{ + using ChangeCollection = eprosima::fastrtps::ResourceLimitedVector; + + //! A vector of DataReader changes belonging to the same instance + ChangeCollection cache_changes; + //! The time when the group will miss the deadline + std::chrono::steady_clock::time_point next_deadline_us; + //! Current view state of the instance + ViewStateKind view_state = ViewStateKind::NEW_VIEW_STATE; + //! Current instance state of the instance + InstanceStateKind instance_state = InstanceStateKind::ALIVE_INSTANCE_STATE; + //! Current disposed generation of the instance + int32_t disposed_generation_count = 0; + //! Current no_writers generation of the instance + int32_t no_writers_generation_count = 0; +}; + +} /* namespace detail */ +} /* namespace dds */ +} /* namespace fastdds */ +} /* namespace eprosima */ + +#endif // _FASTDDS_SUBSCRIBER_HISTORY_DATAREADERCACHECHANGE_HPP_ From 7280d7a2447aa7ae1b348cd9a515af213948d82a Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Fri, 3 Sep 2021 08:17:10 +0200 Subject: [PATCH 03/56] Refs 12400. DataReaderHistory using new types. Signed-off-by: Miguel Company --- .../DataReaderImpl/ReadTakeCommand.hpp | 2 +- .../subscriber/history/DataReaderHistory.cpp | 162 +++++++----------- .../subscriber/history/DataReaderHistory.hpp | 19 +- .../subscriber/history/DataReaderInstance.hpp | 2 +- 4 files changed, 71 insertions(+), 114 deletions(-) diff --git a/src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp b/src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp index 54542d7f5ac..5c6fb6f5bd8 100644 --- a/src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp +++ b/src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp @@ -111,7 +111,7 @@ struct ReadTakeCommand auto it = instance_.second->begin(); while (!finished_ && it != instance_.second->end()) { - CacheChange_t* change = *it; + CacheChange_t* change = it->change; SampleStateKind check; check = change->isRead ? SampleStateKind::READ_SAMPLE_STATE : SampleStateKind::NOT_READ_SAMPLE_STATE; if ((check & states_.sample_states) != 0) diff --git a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp index 5c1adfe6d33..50e56c04e29 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp @@ -37,7 +37,6 @@ namespace detail { using namespace eprosima::fastrtps::rtps; -using eprosima::fastrtps::KeyedChanges; using eprosima::fastrtps::RecursiveTimedMutex; static void get_sample_info( @@ -215,13 +214,13 @@ bool DataReaderHistory::received_change_keep_all_with_key( { // TODO(Miguel C): Should we check unknown_missing_changes_up_to as it is done in received_change_keep_all_no_key? - t_m_Inst_Caches::iterator vit; + InstanceCollection::iterator vit; if (find_key_for_change(a_change, vit)) { - std::vector& instance_changes = vit->second.cache_changes; + DataReaderInstance::ChangeCollection& instance_changes = vit->second.cache_changes; if (instance_changes.size() < static_cast(resource_limited_qos_.max_samples_per_instance)) { - return add_received_change_with_key(a_change, vit->second.cache_changes); + return add_received_change_with_key(a_change, vit->second); } logWarning(SUBSCRIBER, "Change not added due to maximum number of samples per instance"); @@ -234,11 +233,11 @@ bool DataReaderHistory::received_change_keep_last_with_key( CacheChange_t* a_change, size_t /* unknown_missing_changes_up_to */) { - t_m_Inst_Caches::iterator vit; + InstanceCollection::iterator vit; if (find_key_for_change(a_change, vit)) { bool add = false; - std::vector& instance_changes = vit->second.cache_changes; + DataReaderInstance::ChangeCollection& instance_changes = vit->second.cache_changes; if (instance_changes.size() < static_cast(history_qos_.depth)) { add = true; @@ -248,12 +247,12 @@ bool DataReaderHistory::received_change_keep_last_with_key( // Try to substitute the oldest sample. // As the instance should be ordered following the presentation QoS, we can always remove the first one. - add = remove_change_sub(instance_changes.at(0)); + add = remove_change_sub(instance_changes.at(0).change); } if (add) { - return add_received_change_with_key(a_change, instance_changes); + return add_received_change_with_key(a_change, vit->second); } } @@ -263,33 +262,12 @@ bool DataReaderHistory::received_change_keep_last_with_key( bool DataReaderHistory::add_received_change( CacheChange_t* a_change) { - if (m_isHistoryFull) - { - // Discarding the sample. - logWarning(SUBSCRIBER, "Attempting to add Data to Full ReaderHistory: " << type_name_); - return false; - } - - if (add_change(a_change)) - { - if (m_changes.size() == static_cast(m_att.maximumReservedCaches)) - { - m_isHistoryFull = true; - } - - logInfo(SUBSCRIBER, type_name_ - << ": Change " << a_change->sequenceNumber << " added from: " - << a_change->writerGUID; ); - - return true; - } - - return false; + return add_received_change_with_key(a_change, keyed_changes_[c_InstanceHandle_Unknown]); } bool DataReaderHistory::add_received_change_with_key( CacheChange_t* a_change, - std::vector& instance_changes) + DataReaderInstance& instance) { if (m_isHistoryFull) { @@ -306,10 +284,12 @@ bool DataReaderHistory::add_received_change_with_key( } //ADD TO KEY VECTOR - eprosima::utilities::collections::sorted_vector_insert(instance_changes, a_change, - [](const CacheChange_t* lhs, const CacheChange_t* rhs) + DataReaderCacheChange item{ a_change, instance.disposed_generation_count, + instance.no_writers_generation_count }; + eprosima::utilities::collections::sorted_vector_insert(instance.cache_changes, item, + [](const DataReaderCacheChange& lhs, const DataReaderCacheChange& rhs) { - return lhs->sourceTimestamp < rhs->sourceTimestamp; + return lhs.change->sourceTimestamp < rhs.change->sourceTimestamp; }); logInfo(SUBSCRIBER, mp_reader->getGuid().entityId @@ -324,7 +304,7 @@ bool DataReaderHistory::add_received_change_with_key( bool DataReaderHistory::find_key_for_change( CacheChange_t* a_change, - t_m_Inst_Caches::iterator& map_it) + InstanceCollection::iterator& map_it) { if (!a_change->instanceHandle.isDefined() && type_ != nullptr) { @@ -368,9 +348,9 @@ bool DataReaderHistory::get_first_untaken_info( bool DataReaderHistory::find_key( CacheChange_t* a_change, - t_m_Inst_Caches::iterator* vit_out) + InstanceCollection::iterator* vit_out) { - t_m_Inst_Caches::iterator vit; + InstanceCollection::iterator vit; vit = keyed_changes_.find(a_change->instanceHandle); if (vit != keyed_changes_.end()) { @@ -380,7 +360,7 @@ bool DataReaderHistory::find_key( if (keyed_changes_.size() < static_cast(resource_limited_qos_.max_instances)) { - *vit_out = keyed_changes_.insert(std::make_pair(a_change->instanceHandle, KeyedChanges())).first; + *vit_out = keyed_changes_.insert(std::make_pair(a_change->instanceHandle, DataReaderInstance())).first; return true; } else @@ -390,7 +370,7 @@ bool DataReaderHistory::find_key( if (vit->second.cache_changes.size() == 0) { keyed_changes_.erase(vit); - *vit_out = keyed_changes_.insert(std::make_pair(a_change->instanceHandle, KeyedChanges())).first; + *vit_out = keyed_changes_.insert(std::make_pair(a_change->instanceHandle, DataReaderInstance())).first; return true; } } @@ -409,26 +389,24 @@ bool DataReaderHistory::remove_change_sub( } std::lock_guard guard(*mp_mutex); - if (has_keys_) + bool found = false; + InstanceCollection::iterator vit; + if (find_key(change, &vit)) { - bool found = false; - t_m_Inst_Caches::iterator vit; - if (find_key(change, &vit)) + for (auto chit = vit->second.cache_changes.begin(); chit != vit->second.cache_changes.end(); ++chit) { - for (auto chit = vit->second.cache_changes.begin(); chit != vit->second.cache_changes.end(); ++chit) + if (chit->change->sequenceNumber == change->sequenceNumber && + chit->change->writerGUID == change->writerGUID) { - if ((*chit)->sequenceNumber == change->sequenceNumber && (*chit)->writerGUID == change->writerGUID) - { - vit->second.cache_changes.erase(chit); - found = true; - break; - } + vit->second.cache_changes.erase(chit); + found = true; + break; } } - if (!found) - { - logError(SUBSCRIBER, "Change not found on this key, something is wrong"); - } + } + if (!found) + { + logError(SUBSCRIBER, "Change not found on this key, something is wrong"); } if (remove_change(change)) @@ -442,7 +420,7 @@ bool DataReaderHistory::remove_change_sub( bool DataReaderHistory::remove_change_sub( CacheChange_t* change, - iterator& it) + DataReaderInstance::ChangeCollection::iterator& it) { if (mp_reader == nullptr || mp_mutex == nullptr) { @@ -451,27 +429,25 @@ bool DataReaderHistory::remove_change_sub( } std::lock_guard guard(*mp_mutex); - if (has_keys_) + bool found = false; + InstanceCollection::iterator vit; + if (find_key(change, &vit)) { - bool found = false; - t_m_Inst_Caches::iterator vit; - if (find_key(change, &vit)) + for (auto chit = vit->second.cache_changes.begin(); chit != vit->second.cache_changes.end(); ++chit) { - for (auto chit = vit->second.cache_changes.begin(); chit != vit->second.cache_changes.end(); ++chit) + if (chit->change->sequenceNumber == change->sequenceNumber && + chit->change->writerGUID == change->writerGUID) { - if ((*chit)->sequenceNumber == change->sequenceNumber && (*chit)->writerGUID == change->writerGUID) - { - assert(it == chit); - it = vit->second.cache_changes.erase(chit); - found = true; - break; - } + assert(it == chit); + it = vit->second.cache_changes.erase(chit); + found = true; + break; } } - if (!found) - { - logError(SUBSCRIBER, "Change not found on this key, something is wrong"); - } + } + if (!found) + { + logError(SUBSCRIBER, "Change not found on this key, something is wrong"); } const_iterator chit = find_change_nts(change); @@ -482,12 +458,7 @@ bool DataReaderHistory::remove_change_sub( } m_isHistoryFull = false; - iterator ret_it = remove_change_nts(chit); - - if (!has_keys_) - { - it = ret_it; - } + ReaderHistory::remove_change_nts(chit); return true; } @@ -502,13 +473,6 @@ bool DataReaderHistory::set_next_deadline( return false; } std::lock_guard guard(*mp_mutex); - - if (!has_keys_) - { - next_deadline_us_ = next_deadline_us; - return true; - } - if (keyed_changes_.find(handle) == keyed_changes_.end()) { return false; @@ -528,18 +492,11 @@ bool DataReaderHistory::get_next_deadline( return false; } std::lock_guard guard(*mp_mutex); - - if (!has_keys_) - { - next_deadline_us = next_deadline_us_; - return true; - } - auto min = std::min_element(keyed_changes_.begin(), keyed_changes_.end(), []( - const std::pair& lhs, - const std::pair& rhs) + const std::pair& lhs, + const std::pair& rhs) { return lhs.second.next_deadline_us < rhs.second.next_deadline_us; }); @@ -573,10 +530,10 @@ std::pair DataReaderHistory::lookup_inst // Looking for the first instance, return the ficticious one containing all changes InstanceHandle_t tmp; tmp.value[0] = 1; - return { true, {tmp, &m_changes} }; + return { true, {tmp, &keyed_changes_.begin()->second.cache_changes} }; } - t_m_Inst_Caches::iterator it; + InstanceCollection::iterator it; if (exact) { @@ -584,7 +541,7 @@ std::pair DataReaderHistory::lookup_inst } else { - auto comp = [](const InstanceHandle_t& h, const std::pair& it) + auto comp = [](const InstanceHandle_t& h, const std::pair& it) { return h < it.first; }; @@ -602,12 +559,10 @@ ReaderHistory::iterator DataReaderHistory::remove_change_nts( ReaderHistory::const_iterator removal, bool release) { - CacheChange_t* p_sample = nullptr; - - if ( removal != changesEnd() - && (p_sample = *removal)->instanceHandle.isDefined() - && has_keys_) + if (removal != changesEnd()) { + CacheChange_t* p_sample = *removal; + // clean any references to this CacheChange in the key state collection auto it = keyed_changes_.find(p_sample->instanceHandle); @@ -615,7 +570,10 @@ ReaderHistory::iterator DataReaderHistory::remove_change_nts( assert(it != keyed_changes_.end()); auto& c = it->second.cache_changes; - c.erase(std::remove(c.begin(), c.end(), p_sample), c.end()); + c.erase(std::remove_if(c.begin(), c.end(), [p_sample](DataReaderCacheChange& elem) + { + return elem.change == p_sample; + }), c.end()); } // call the base class diff --git a/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp b/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp index 236ba1f767b..a76e44809f7 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp @@ -38,9 +38,10 @@ #include #include -#include #include +#include "DataReaderInstance.hpp" + namespace eprosima { namespace fastdds { namespace dds { @@ -57,7 +58,7 @@ class DataReaderHistory : public eprosima::fastrtps::rtps::ReaderHistory using InstanceHandle_t = eprosima::fastrtps::rtps::InstanceHandle_t; using CacheChange_t = eprosima::fastrtps::rtps::CacheChange_t; - using instance_info = std::pair*>; + using instance_info = std::pair; /** * Constructor. Requires information about the subscriber. @@ -120,7 +121,7 @@ class DataReaderHistory : public eprosima::fastrtps::rtps::ReaderHistory */ bool remove_change_sub( CacheChange_t* change, - iterator& it); + DataReaderInstance::ChangeCollection::iterator& it); /** * @brief A method to set the next deadline for the given instance @@ -163,12 +164,10 @@ class DataReaderHistory : public eprosima::fastrtps::rtps::ReaderHistory private: - using t_m_Inst_Caches = std::map; + using InstanceCollection = std::map; //!Map where keys are instance handles and values vectors of cache changes - t_m_Inst_Caches keyed_changes_; - //!Time point when the next deadline will occur (only used for topics with no key) - std::chrono::steady_clock::time_point next_deadline_us_; + InstanceCollection keyed_changes_; //!HistoryQosPolicy values. HistoryQosPolicy history_qos_; //!ResourceLimitsQosPolicy values. @@ -196,7 +195,7 @@ class DataReaderHistory : public eprosima::fastrtps::rtps::ReaderHistory */ bool find_key( CacheChange_t* a_change, - t_m_Inst_Caches::iterator* map_it); + InstanceCollection::iterator* map_it); /** * @brief Method that finds a key in m_keyedChanges or tries to add it if not found @@ -206,7 +205,7 @@ class DataReaderHistory : public eprosima::fastrtps::rtps::ReaderHistory */ bool find_key_for_change( CacheChange_t* a_change, - t_m_Inst_Caches::iterator& map_it); + InstanceCollection::iterator& map_it); /** * @name Variants of incoming change processing. @@ -238,7 +237,7 @@ class DataReaderHistory : public eprosima::fastrtps::rtps::ReaderHistory bool add_received_change_with_key( CacheChange_t* a_change, - std::vector& instance_changes); + DataReaderInstance& instance); }; } // namespace detail diff --git a/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp b/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp index 76d218a163a..e33587d31b3 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp @@ -37,7 +37,7 @@ namespace detail { /// Book-keeping information for an instance struct DataReaderInstance { - using ChangeCollection = eprosima::fastrtps::ResourceLimitedVector; + using ChangeCollection = std::vector; //! A vector of DataReader changes belonging to the same instance ChangeCollection cache_changes; From b2b1fae87392b70774799a0136a1f2a633f3da8f Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Tue, 7 Sep 2021 10:58:16 +0200 Subject: [PATCH 04/56] Refs 12400. Removing unnecessary method from SubscriberHistory. Signed-off-by: Miguel Company --- .../fastrtps/subscriber/SubscriberHistory.h | 21 -------- .../subscriber/SubscriberHistory.cpp | 52 ------------------- 2 files changed, 73 deletions(-) diff --git a/include/fastrtps/subscriber/SubscriberHistory.h b/include/fastrtps/subscriber/SubscriberHistory.h index d0d1f1fdf2b..be94ed6a291 100644 --- a/include/fastrtps/subscriber/SubscriberHistory.h +++ b/include/fastrtps/subscriber/SubscriberHistory.h @@ -44,8 +44,6 @@ class SubscriberHistory : public rtps::ReaderHistory { public: - using instance_info = std::pair*>; - /** * Constructor. Requires information about the subscriber. * @param topic_att TopicAttributes. @@ -179,25 +177,6 @@ class SubscriberHistory : public rtps::ReaderHistory rtps::InstanceHandle_t& handle, std::chrono::steady_clock::time_point& next_deadline_us); - /** - * @brief Get the list of changes corresponding to an instance handle. - * @param handle The handle to the instance. - * @param exact Indicates if the handle should match exactly (true) or if the first instance greater than the - * input handle should be returned. - * @return A pair where: - * - @c first is a boolean indicating if an instance was found - * - @c second is a pair where: - * - @c first is the handle of the returned instance - * - @c second is a pointer to a std::vector with the list of changes for the - * returned instance - * - * @remarks When used on a NO_KEY topic, an instance will only be returned when called with - * `handle = HANDLE_NIL` and `exact = false`. - */ - std::pair lookup_instance( - const rtps::InstanceHandle_t& handle, - bool exact); - private: using t_m_Inst_Caches = std::map; diff --git a/src/cpp/fastrtps_deprecated/subscriber/SubscriberHistory.cpp b/src/cpp/fastrtps_deprecated/subscriber/SubscriberHistory.cpp index 9de26f80dd5..39a7b4542d6 100644 --- a/src/cpp/fastrtps_deprecated/subscriber/SubscriberHistory.cpp +++ b/src/cpp/fastrtps_deprecated/subscriber/SubscriberHistory.cpp @@ -693,58 +693,6 @@ bool SubscriberHistory::get_next_deadline( return false; } -std::pair SubscriberHistory::lookup_instance( - const InstanceHandle_t& handle, - bool exact) -{ - if (topic_att_.getTopicKind() == NO_KEY) - { - if (handle.isDefined()) - { - // NO_KEY topics can only return the ficticious instance. - // Execution can only get here for two reasons: - // - Looking for a specific instance (exact = true) - // - Looking for the next instance to the ficticious one (exact = false) - // In both cases, no instance should be returned - return { false, {InstanceHandle_t(), nullptr} }; - } - else - { - if (exact) - { - // Looking for HANDLE_NIL, nothing to return - return { false, {InstanceHandle_t(), nullptr} }; - } - - // Looking for the first instance, return the ficticious one containing all changes - InstanceHandle_t tmp; - tmp.value[0] = 1; - return { true, {tmp, &m_changes} }; - } - } - - t_m_Inst_Caches::iterator it; - - if (exact) - { - it = keyed_changes_.find(handle); - } - else - { - auto comp = [](const InstanceHandle_t& h, const std::pair& it) - { - return h < it.first; - }; - it = std::upper_bound(keyed_changes_.begin(), keyed_changes_.end(), handle, comp); - } - - if (it != keyed_changes_.end()) - { - return { true, {it->first, &(it->second.cache_changes)} }; - } - return { false, {InstanceHandle_t(), nullptr} }; -} - ReaderHistory::iterator SubscriberHistory::remove_change_nts( ReaderHistory::const_iterator removal, bool release) From 4ee4f024f42b521a50dc2fe85a1fd62f974263fd Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Tue, 7 Sep 2021 11:05:38 +0200 Subject: [PATCH 05/56] Refs 12400. ReadTakeCommand receives full instance information. Signed-off-by: Miguel Company --- .../fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp | 6 +++--- src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp | 4 ++-- src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp b/src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp index 5c6fb6f5bd8..a48a9010e10 100644 --- a/src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp +++ b/src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp @@ -64,7 +64,7 @@ struct ReadTakeCommand SampleInfoSeq& sample_infos, int32_t max_samples, const StateFilter& states, - history_type::instance_info instance, + const history_type::instance_info& instance, bool single_instance = false) : type_(reader.type_) , loan_manager_(reader.loan_manager_) @@ -108,8 +108,8 @@ struct ReadTakeCommand // Traverse changes on current instance bool ret_val = false; LoanableCollection::size_type first_slot = current_slot_; - auto it = instance_.second->begin(); - while (!finished_ && it != instance_.second->end()) + auto it = instance_.second->cache_changes.begin(); + while (!finished_ && it != instance_.second->cache_changes.end()) { CacheChange_t* change = it->change; SampleStateKind check; diff --git a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp index 50e56c04e29..7a6c4573aaf 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp @@ -530,7 +530,7 @@ std::pair DataReaderHistory::lookup_inst // Looking for the first instance, return the ficticious one containing all changes InstanceHandle_t tmp; tmp.value[0] = 1; - return { true, {tmp, &keyed_changes_.begin()->second.cache_changes} }; + return { true, {tmp, &keyed_changes_.begin()->second} }; } InstanceCollection::iterator it; @@ -550,7 +550,7 @@ std::pair DataReaderHistory::lookup_inst if (it != keyed_changes_.end()) { - return { true, {it->first, &(it->second.cache_changes)} }; + return { true, {it->first, &(it->second)} }; } return { false, {InstanceHandle_t(), nullptr} }; } diff --git a/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp b/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp index a76e44809f7..2636ec70715 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp @@ -58,7 +58,7 @@ class DataReaderHistory : public eprosima::fastrtps::rtps::ReaderHistory using InstanceHandle_t = eprosima::fastrtps::rtps::InstanceHandle_t; using CacheChange_t = eprosima::fastrtps::rtps::CacheChange_t; - using instance_info = std::pair; + using instance_info = std::pair; /** * Constructor. Requires information about the subscriber. From e2ef28cb4ee5155ca3b46bd6926991c04e8a6473 Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Tue, 7 Sep 2021 11:11:15 +0200 Subject: [PATCH 06/56] Refs 12400. ReadTakeCommand checks for instance states. Signed-off-by: Miguel Company --- .../fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp b/src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp index a48a9010e10..02a04a1ea2c 100644 --- a/src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp +++ b/src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp @@ -243,10 +243,10 @@ struct ReadTakeCommand bool is_current_instance_valid() { - // We are not implementing instance_state or view_state yet, so all instances will be considered to have - // a valid state. In the future this should check instance_state against states_.instance_states and - // view_state against states_.view_states - return true; + // Check instance_state against states_.instance_states and view_state against states_.view_states + auto instance_state = instance_.second->instance_state; + auto view_state = instance_.second->view_state; + return (0 != (states_.instance_states & instance_state)) && (0 != (states_.view_states & view_state)); } bool next_instance() From da93a9d24331f99d52dc5ae3f514fab4d1804f95 Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Tue, 7 Sep 2021 11:23:42 +0200 Subject: [PATCH 07/56] Refs 12400. ReadTakeCommand fills sample info from instance data. Signed-off-by: Miguel Company --- .../DataReaderImpl/ReadTakeCommand.hpp | 47 +++++++++---------- 1 file changed, 21 insertions(+), 26 deletions(-) diff --git a/src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp b/src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp index 02a04a1ea2c..37f98fb2399 100644 --- a/src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp +++ b/src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp @@ -142,10 +142,9 @@ struct ReadTakeCommand // in the future also if (!is_future_change) { - // Add sample and info to collections ReturnCode_t previous_return_value = return_value_; - bool added = add_sample(change, remove_change); + bool added = add_sample(*it, remove_change); reader_->end_sample_access_nts(change, wp, added); // Check if the payload is dirty @@ -270,7 +269,7 @@ struct ReadTakeCommand } bool add_sample( - CacheChange_t* change, + const DataReaderCacheChange& item, bool& deserialization_error) { bool ret_val = false; @@ -284,10 +283,10 @@ struct ReadTakeCommand sample_infos_.length(new_len); // Add information - generate_info(change); + generate_info(item); if (sample_infos_[current_slot_].valid_data) { - if (!deserialize_sample(change)) + if (!deserialize_sample(item.change)) { // Decrement length of collections data_values_.length(current_slot_); @@ -330,46 +329,42 @@ struct ReadTakeCommand } void generate_info( - CacheChange_t* change) + const DataReaderCacheChange& item) { // Loan when necessary if (!sample_infos_.has_ownership()) { - SampleInfo* item = info_pool_.get_item(); - assert(item != nullptr); - const_cast(sample_infos_.buffer())[current_slot_] = item; + SampleInfo* pool_item = info_pool_.get_item(); + assert(pool_item != nullptr); + const_cast(sample_infos_.buffer())[current_slot_] = pool_item; } SampleInfo& info = sample_infos_[current_slot_]; - info.sample_state = change->isRead ? READ_SAMPLE_STATE : NOT_READ_SAMPLE_STATE; - info.view_state = NOT_NEW_VIEW_STATE; - info.disposed_generation_count = 0; - info.no_writers_generation_count = 1; + info.sample_state = item.change->isRead ? READ_SAMPLE_STATE : NOT_READ_SAMPLE_STATE; + info.instance_state = instance_.second->instance_state; + info.view_state = instance_.second->view_state; + info.disposed_generation_count = item.disposed_generation_count; + info.no_writers_generation_count = item.no_writers_generation_count; info.sample_rank = 0; info.generation_rank = 0; info.absoulte_generation_rank = 0; - info.source_timestamp = change->sourceTimestamp; - info.reception_timestamp = change->reader_info.receptionTimestamp; + info.source_timestamp = item.change->sourceTimestamp; + info.reception_timestamp = item.change->reader_info.receptionTimestamp; info.instance_handle = handle_; - info.publication_handle = InstanceHandle_t(change->writerGUID); - info.sample_identity.writer_guid(change->writerGUID); - info.sample_identity.sequence_number(change->sequenceNumber); - info.related_sample_identity = change->write_params.sample_identity(); + info.publication_handle = InstanceHandle_t(item.change->writerGUID); + info.sample_identity.writer_guid(item.change->writerGUID); + info.sample_identity.sequence_number(item.change->sequenceNumber); + info.related_sample_identity = item.change->write_params.sample_identity(); info.valid_data = true; - switch (change->kind) + switch (item.change->kind) { - case eprosima::fastrtps::rtps::ALIVE: - info.instance_state = ALIVE_INSTANCE_STATE; - break; case eprosima::fastrtps::rtps::NOT_ALIVE_DISPOSED: case eprosima::fastrtps::rtps::NOT_ALIVE_DISPOSED_UNREGISTERED: - info.instance_state = NOT_ALIVE_DISPOSED_INSTANCE_STATE; info.valid_data = false; break; + case eprosima::fastrtps::rtps::ALIVE: default: - //TODO [ILG] change this if the other kinds ever get implemented - info.instance_state = ALIVE_INSTANCE_STATE; break; } } From 0c1f59f89989f203e6b07d263aca9b35d5e9ebf9 Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Tue, 7 Sep 2021 12:40:47 +0200 Subject: [PATCH 08/56] Refs 12400. Added insert method to ResourceLimitedVector. Signed-off-by: Miguel Company --- .../collections/ResourceLimitedVector.hpp | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/include/fastrtps/utils/collections/ResourceLimitedVector.hpp b/include/fastrtps/utils/collections/ResourceLimitedVector.hpp index 91aa80a3c12..f881f13d388 100644 --- a/include/fastrtps/utils/collections/ResourceLimitedVector.hpp +++ b/include/fastrtps/utils/collections/ResourceLimitedVector.hpp @@ -119,6 +119,46 @@ class ResourceLimitedVector return *this; } + /** + * Insert value before pos. + * + * @param pos iterator before which the content will be inserted. pos may be the end() iterator. + * @param value element value to insert. + * + * @return Iterator pointing to the inserted value. end() if insertion couldn't be done due to collection limits. + */ + iterator insert( + const_iterator pos, + const value_type& value) + { + if (capacity() >= configuration_.maximum) + { + return end(); + } + + return collection_.insert(pos, value); + } + + /** + * Insert value before pos. + * + * @param pos iterator before which the content will be inserted. pos may be the end() iterator. + * @param value element value to insert. + * + * @return Iterator pointing to the inserted value. end() if insertion couldn't be done due to collection limits. + */ + iterator insert( + const_iterator pos, + value_type&& value) + { + if (capacity() >= configuration_.maximum) + { + return end(); + } + + return collection_.insert(pos, value); + } + /** * Add element at the end. * From 817dfd4df5bab86fa4ebbb9c357ec197be268615 Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Tue, 7 Sep 2021 15:03:29 +0200 Subject: [PATCH 09/56] Refs 12400. DataReaderInstance uses ResourceLimitedVector. Signed-off-by: Miguel Company --- src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp b/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp index e33587d31b3..76d218a163a 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp @@ -37,7 +37,7 @@ namespace detail { /// Book-keeping information for an instance struct DataReaderInstance { - using ChangeCollection = std::vector; + using ChangeCollection = eprosima::fastrtps::ResourceLimitedVector; //! A vector of DataReader changes belonging to the same instance ChangeCollection cache_changes; From 860b93270b1944a49bc83bfd2ea71d3a96247d25 Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Wed, 8 Sep 2021 10:50:24 +0200 Subject: [PATCH 10/56] Refs 12400. get_first_untaken_info takes sample info from instance data. Signed-off-by: Miguel Company --- .../DataReaderImpl/ReadTakeCommand.hpp | 62 +++++++++++-------- .../subscriber/history/DataReaderHistory.cpp | 47 ++++---------- 2 files changed, 46 insertions(+), 63 deletions(-) diff --git a/src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp b/src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp index 37f98fb2399..d86d4207f1d 100644 --- a/src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp +++ b/src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp @@ -207,6 +207,40 @@ struct ReadTakeCommand return return_value_; } + static void generate_info( + SampleInfo& info, + const DataReaderInstance& instance, + const DataReaderCacheChange& item) + { + info.sample_state = item.change->isRead ? READ_SAMPLE_STATE : NOT_READ_SAMPLE_STATE; + info.instance_state = instance.instance_state; + info.view_state = instance.view_state; + info.disposed_generation_count = item.disposed_generation_count; + info.no_writers_generation_count = item.no_writers_generation_count; + info.sample_rank = 0; + info.generation_rank = 0; + info.absoulte_generation_rank = 0; + info.source_timestamp = item.change->sourceTimestamp; + info.reception_timestamp = item.change->reader_info.receptionTimestamp; + info.instance_handle = item.change->instanceHandle; + info.publication_handle = InstanceHandle_t(item.change->writerGUID); + info.sample_identity.writer_guid(item.change->writerGUID); + info.sample_identity.sequence_number(item.change->sequenceNumber); + info.related_sample_identity = item.change->write_params.sample_identity(); + info.valid_data = true; + + switch (item.change->kind) + { + case eprosima::fastrtps::rtps::NOT_ALIVE_DISPOSED: + case eprosima::fastrtps::rtps::NOT_ALIVE_DISPOSED_UNREGISTERED: + info.valid_data = false; + break; + case eprosima::fastrtps::rtps::ALIVE: + default: + break; + } + } + private: const TypeSupport& type_; @@ -340,33 +374,7 @@ struct ReadTakeCommand } SampleInfo& info = sample_infos_[current_slot_]; - info.sample_state = item.change->isRead ? READ_SAMPLE_STATE : NOT_READ_SAMPLE_STATE; - info.instance_state = instance_.second->instance_state; - info.view_state = instance_.second->view_state; - info.disposed_generation_count = item.disposed_generation_count; - info.no_writers_generation_count = item.no_writers_generation_count; - info.sample_rank = 0; - info.generation_rank = 0; - info.absoulte_generation_rank = 0; - info.source_timestamp = item.change->sourceTimestamp; - info.reception_timestamp = item.change->reader_info.receptionTimestamp; - info.instance_handle = handle_; - info.publication_handle = InstanceHandle_t(item.change->writerGUID); - info.sample_identity.writer_guid(item.change->writerGUID); - info.sample_identity.sequence_number(item.change->sequenceNumber); - info.related_sample_identity = item.change->write_params.sample_identity(); - info.valid_data = true; - - switch (item.change->kind) - { - case eprosima::fastrtps::rtps::NOT_ALIVE_DISPOSED: - case eprosima::fastrtps::rtps::NOT_ALIVE_DISPOSED_UNREGISTERED: - info.valid_data = false; - break; - case eprosima::fastrtps::rtps::ALIVE: - default: - break; - } + generate_info(info, *instance_.second, item); } bool check_datasharing_validity( diff --git a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp index 7a6c4573aaf..bcbbdc2eb30 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp @@ -27,6 +27,7 @@ #include #include +#include #include #include @@ -39,41 +40,6 @@ using namespace eprosima::fastrtps::rtps; using eprosima::fastrtps::RecursiveTimedMutex; -static void get_sample_info( - SampleInfo& info, - CacheChange_t* change) -{ - info.sample_state = NOT_READ_SAMPLE_STATE; - info.view_state = NOT_NEW_VIEW_STATE; - info.disposed_generation_count = 0; - info.no_writers_generation_count = 1; - info.sample_rank = 0; - info.generation_rank = 0; - info.absoulte_generation_rank = 0; - info.source_timestamp = change->sourceTimestamp; - info.reception_timestamp = change->reader_info.receptionTimestamp; - info.instance_handle = change->instanceHandle; - info.publication_handle = fastrtps::rtps::InstanceHandle_t(change->writerGUID); - info.sample_identity.writer_guid(change->writerGUID); - info.sample_identity.sequence_number(change->sequenceNumber); - info.related_sample_identity = change->write_params.sample_identity(); - info.valid_data = change->kind == eprosima::fastrtps::rtps::ALIVE; - - switch (change->kind) - { - case eprosima::fastrtps::rtps::ALIVE: - info.instance_state = ALIVE_INSTANCE_STATE; - break; - case eprosima::fastrtps::rtps::NOT_ALIVE_DISPOSED: - info.instance_state = NOT_ALIVE_DISPOSED_INSTANCE_STATE; - break; - default: - //TODO [ILG] change this if the other kinds ever get implemented - info.instance_state = ALIVE_INSTANCE_STATE; - break; - } -} - static HistoryAttributes to_history_attributes( const TypeSupport& type, const DataReaderQos& qos) @@ -338,7 +304,16 @@ bool DataReaderHistory::get_first_untaken_info( WriterProxy* wp = nullptr; if (mp_reader->nextUntakenCache(&change, &wp)) { - get_sample_info(info, change); + auto it = keyed_changes_.find(change->instanceHandle); + assert(it != keyed_changes_.end()); + auto& instance_changes = it->second.cache_changes; + auto item = + std::find_if(instance_changes.cbegin(), instance_changes.cend(), + [change](const DataReaderCacheChange& v) + { + return v.change == change; + }); + ReadTakeCommand::generate_info(info, it->second, *item); mp_reader->change_read_by_user(change, wp, false); return true; } From f868b4d27ca70b9e73cd4bbbddb768f2056636a6 Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Wed, 8 Sep 2021 11:13:12 +0200 Subject: [PATCH 11/56] Refs 12400. Discard received change when older than oldest. Signed-off-by: Miguel Company --- .../subscriber/history/DataReaderHistory.cpp | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp index bcbbdc2eb30..a02c0f192e4 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp @@ -161,9 +161,15 @@ bool DataReaderHistory::received_change_keep_last_no_key( else { // Try to substitute the oldest sample. + CacheChange_t* first_change = m_changes.at(0); + if (a_change->sourceTimestamp < first_change->sourceTimestamp) + { + // Received change is older than oldest, and should be discarded + return true; + } - // As the history should be ordered following the presentation QoS, we can always remove the first one. - add = remove_change_sub(m_changes.at(0)); + // As the history is ordered by source timestamp, we can always remove the first one. + add = remove_change_sub(first_change); } if (add) @@ -211,9 +217,15 @@ bool DataReaderHistory::received_change_keep_last_with_key( else { // Try to substitute the oldest sample. + CacheChange_t* first_change = instance_changes.at(0).change; + if (a_change->sourceTimestamp < first_change->sourceTimestamp) + { + // Received change is older than oldest, and should be discarded + return true; + } - // As the instance should be ordered following the presentation QoS, we can always remove the first one. - add = remove_change_sub(instance_changes.at(0).change); + // As the instance is ordered by source timestamp, we can always remove the first one. + add = remove_change_sub(first_change); } if (add) From bb5e29bb8462499488f81daac2c747471daa06a8 Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Wed, 8 Sep 2021 11:32:14 +0200 Subject: [PATCH 12/56] Refs 12400. Fixing KEEP_ALL with keys. Signed-off-by: Miguel Company --- src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp index a02c0f192e4..a4488bcf193 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp @@ -182,20 +182,19 @@ bool DataReaderHistory::received_change_keep_last_no_key( bool DataReaderHistory::received_change_keep_all_with_key( CacheChange_t* a_change, - size_t /* unknown_missing_changes_up_to */ ) + size_t unknown_missing_changes_up_to) { - // TODO(Miguel C): Should we check unknown_missing_changes_up_to as it is done in received_change_keep_all_no_key? - InstanceCollection::iterator vit; if (find_key_for_change(a_change, vit)) { DataReaderInstance::ChangeCollection& instance_changes = vit->second.cache_changes; - if (instance_changes.size() < static_cast(resource_limited_qos_.max_samples_per_instance)) + size_t total_size = instance_changes.size() + unknown_missing_changes_up_to; + if (total_size < static_cast(resource_limited_qos_.max_samples_per_instance)) { return add_received_change_with_key(a_change, vit->second); } - logWarning(SUBSCRIBER, "Change not added due to maximum number of samples per instance"); + logInfo(SUBSCRIBER, "Change not added due to maximum number of samples per instance"); } return false; From 0b2b047b9572a4bb581261f43a1bcf61a3b758e2 Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Wed, 8 Sep 2021 12:58:02 +0200 Subject: [PATCH 13/56] Refs 12400. Refactor to always use instances. Signed-off-by: Miguel Company --- .../subscriber/history/DataReaderHistory.cpp | 150 +++++++----------- .../subscriber/history/DataReaderHistory.hpp | 28 ++-- 2 files changed, 68 insertions(+), 110 deletions(-) diff --git a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp index a4488bcf193..926205d4fde 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp @@ -77,14 +77,19 @@ DataReaderHistory::DataReaderHistory( , type_(type.get()) , get_key_object_(nullptr) { + if (resource_limited_qos_.max_samples == 0) + { + resource_limited_qos_.max_samples = std::numeric_limits::max(); + } + if (type_->m_isGetKeyDefined) { get_key_object_ = type_->createData(); } - - if (resource_limited_qos_.max_samples == 0) + else { - resource_limited_qos_.max_samples = std::numeric_limits::max(); + resource_limited_qos_.max_instances = 1; + resource_limited_qos_.max_samples_per_instance = resource_limited_qos_.max_samples; } if (resource_limited_qos_.max_instances == 0) @@ -100,17 +105,43 @@ DataReaderHistory::DataReaderHistory( using std::placeholders::_1; using std::placeholders::_2; + receive_fn_ = qos.history().kind == KEEP_ALL_HISTORY_QOS ? + std::bind(&DataReaderHistory::received_change_keep_all, this, _1, _2) : + std::bind(&DataReaderHistory::received_change_keep_last, this, _1, _2); + if (!has_keys_) { - receive_fn_ = qos.history().kind == KEEP_ALL_HISTORY_QOS ? - std::bind(&DataReaderHistory::received_change_keep_all_no_key, this, _1, _2) : - std::bind(&DataReaderHistory::received_change_keep_last_no_key, this, _1, _2); + compute_key_for_change_fn_ = [](CacheChange_t* change) + { + change->instanceHandle = c_InstanceHandle_Unknown; + return true; + }; } else { - receive_fn_ = qos.history().kind == KEEP_ALL_HISTORY_QOS ? - std::bind(&DataReaderHistory::received_change_keep_all_with_key, this, _1, _2) : - std::bind(&DataReaderHistory::received_change_keep_last_with_key, this, _1, _2); + compute_key_for_change_fn_ = + [this](CacheChange_t* a_change) + { + if (a_change->instanceHandle.isDefined()) + { + return true; + } + + if (type_ != nullptr) + { + logInfo(SUBSCRIBER, "Getting Key of change with no Key transmitted"); + type_->deserialize(&a_change->serializedPayload, get_key_object_); + bool is_key_protected = false; +#if HAVE_SECURITY + is_key_protected = mp_reader->getAttributes().security_attributes().is_key_protected; +#endif // if HAVE_SECURITY + return type_->getKey(get_key_object_, &a_change->instanceHandle, is_key_protected); + } + + logWarning(SUBSCRIBER, "NO KEY in topic: " << topic_name_ + << " and no method to obtain it"; ); + return false; + }; } } @@ -136,51 +167,7 @@ bool DataReaderHistory::received_change( return receive_fn_(a_change, unknown_missing_changes_up_to); } -bool DataReaderHistory::received_change_keep_all_no_key( - CacheChange_t* a_change, - size_t unknown_missing_changes_up_to) -{ - // TODO(Ricardo) Check - if (m_changes.size() + unknown_missing_changes_up_to < static_cast(resource_limited_qos_.max_samples)) - { - return add_received_change(a_change); - } - - return false; -} - -bool DataReaderHistory::received_change_keep_last_no_key( - CacheChange_t* a_change, - size_t /* unknown_missing_changes_up_to */ ) -{ - bool add = false; - if (m_changes.size() < static_cast(history_qos_.depth)) - { - add = true; - } - else - { - // Try to substitute the oldest sample. - CacheChange_t* first_change = m_changes.at(0); - if (a_change->sourceTimestamp < first_change->sourceTimestamp) - { - // Received change is older than oldest, and should be discarded - return true; - } - - // As the history is ordered by source timestamp, we can always remove the first one. - add = remove_change_sub(first_change); - } - - if (add) - { - return add_received_change(a_change); - } - - return false; -} - -bool DataReaderHistory::received_change_keep_all_with_key( +bool DataReaderHistory::received_change_keep_all( CacheChange_t* a_change, size_t unknown_missing_changes_up_to) { @@ -200,7 +187,7 @@ bool DataReaderHistory::received_change_keep_all_with_key( return false; } -bool DataReaderHistory::received_change_keep_last_with_key( +bool DataReaderHistory::received_change_keep_last( CacheChange_t* a_change, size_t /* unknown_missing_changes_up_to */) { @@ -283,27 +270,7 @@ bool DataReaderHistory::find_key_for_change( CacheChange_t* a_change, InstanceCollection::iterator& map_it) { - if (!a_change->instanceHandle.isDefined() && type_ != nullptr) - { - logInfo(SUBSCRIBER, "Getting Key of change with no Key transmitted"); - type_->deserialize(&a_change->serializedPayload, get_key_object_); - bool is_key_protected = false; -#if HAVE_SECURITY - is_key_protected = mp_reader->getAttributes().security_attributes().is_key_protected; -#endif // if HAVE_SECURITY - if (!type_->getKey(get_key_object_, &a_change->instanceHandle, is_key_protected)) - { - return false; - } - } - else if (!a_change->instanceHandle.isDefined()) - { - logWarning(SUBSCRIBER, "NO KEY in topic: " << topic_name_ - << " and no method to obtain it"; ); - return false; - } - - return find_key(a_change, &map_it); + return compute_key_for_change_fn_(a_change) && find_key(a_change->instanceHandle, map_it); } bool DataReaderHistory::get_first_untaken_info( @@ -333,35 +300,34 @@ bool DataReaderHistory::get_first_untaken_info( } bool DataReaderHistory::find_key( - CacheChange_t* a_change, - InstanceCollection::iterator* vit_out) + const InstanceHandle_t& handle, + InstanceCollection::iterator& vit_out) { InstanceCollection::iterator vit; - vit = keyed_changes_.find(a_change->instanceHandle); + vit = keyed_changes_.find(handle); if (vit != keyed_changes_.end()) { - *vit_out = vit; + vit_out = vit; return true; } if (keyed_changes_.size() < static_cast(resource_limited_qos_.max_instances)) { - *vit_out = keyed_changes_.insert(std::make_pair(a_change->instanceHandle, DataReaderInstance())).first; + vit_out = keyed_changes_.insert(std::make_pair(handle, DataReaderInstance())).first; return true; } - else + + for (vit = keyed_changes_.begin(); vit != keyed_changes_.end(); ++vit) { - for (vit = keyed_changes_.begin(); vit != keyed_changes_.end(); ++vit) + if (vit->second.cache_changes.size() == 0) { - if (vit->second.cache_changes.size() == 0) - { - keyed_changes_.erase(vit); - *vit_out = keyed_changes_.insert(std::make_pair(a_change->instanceHandle, DataReaderInstance())).first; - return true; - } + keyed_changes_.erase(vit); + vit_out = keyed_changes_.insert(std::make_pair(handle, DataReaderInstance())).first; + return true; } - logWarning(SUBSCRIBER, "History has reached the maximum number of instances"); } + + logWarning(SUBSCRIBER, "History has reached the maximum number of instances"); return false; } @@ -377,7 +343,7 @@ bool DataReaderHistory::remove_change_sub( std::lock_guard guard(*mp_mutex); bool found = false; InstanceCollection::iterator vit; - if (find_key(change, &vit)) + if (find_key(change->instanceHandle, vit)) { for (auto chit = vit->second.cache_changes.begin(); chit != vit->second.cache_changes.end(); ++chit) { @@ -417,7 +383,7 @@ bool DataReaderHistory::remove_change_sub( std::lock_guard guard(*mp_mutex); bool found = false; InstanceCollection::iterator vit; - if (find_key(change, &vit)) + if (find_key(change->instanceHandle, vit)) { for (auto chit = vit->second.cache_changes.begin(); chit != vit->second.cache_changes.end(); ++chit) { diff --git a/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp b/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp index 2636ec70715..225c5e9140a 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp @@ -61,12 +61,10 @@ class DataReaderHistory : public eprosima::fastrtps::rtps::ReaderHistory using instance_info = std::pair; /** - * Constructor. Requires information about the subscriber. - * @param topic_att TopicAttributes. - * @param type TopicDataType. - * @param qos ReaderQoS policy. - * @param payloadMax Maximum payload size per change. - * @param mempolicy Set whether the payloads ccan dynamically resized or not. + * Constructor. Requires information about the DataReader. + * @param type Type information. Needed to know if the type is keyed, as long as the maximum serialized size. + * @param topic Topic description. Topic and type name are used on debug messages. + * @param qos DataReaderQoS policy. History related limits are taken from here. */ DataReaderHistory( const TypeSupport& type, @@ -186,6 +184,8 @@ class DataReaderHistory : public eprosima::fastrtps::rtps::ReaderHistory /// Function processing a received change std::function receive_fn_; + /// Function to compute the instance handle of a received change + std::function compute_key_for_change_fn_; /** * @brief Method that finds a key in m_keyedChanges or tries to add it if not found @@ -194,8 +194,8 @@ class DataReaderHistory : public eprosima::fastrtps::rtps::ReaderHistory * @return True if it was found or could be added to the map */ bool find_key( - CacheChange_t* a_change, - InstanceCollection::iterator* map_it); + const InstanceHandle_t& handle, + InstanceCollection::iterator& map_it); /** * @brief Method that finds a key in m_keyedChanges or tries to add it if not found @@ -215,19 +215,11 @@ class DataReaderHistory : public eprosima::fastrtps::rtps::ReaderHistory * @return */ ///@{ - bool received_change_keep_all_no_key( - CacheChange_t* change, - size_t unknown_missing_changes_up_to); - - bool received_change_keep_last_no_key( - CacheChange_t* change, - size_t unknown_missing_changes_up_to); - - bool received_change_keep_all_with_key( + bool received_change_keep_all( CacheChange_t* change, size_t unknown_missing_changes_up_to); - bool received_change_keep_last_with_key( + bool received_change_keep_last( CacheChange_t* change, size_t unknown_missing_changes_up_to); ///@} From 9c69a9a9a3f45327d3611bcc51250ffa442277df Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Tue, 26 Oct 2021 14:40:01 +0200 Subject: [PATCH 14/56] Refs 12400. Basic structure for update instance state. Signed-off-by: Miguel Company --- src/cpp/fastdds/subscriber/DataReaderImpl.cpp | 6 +- .../subscriber/history/DataReaderHistory.cpp | 10 ++++ .../subscriber/history/DataReaderHistory.hpp | 3 + .../subscriber/history/DataReaderInstance.hpp | 57 +++++++++++++++++++ 4 files changed, 74 insertions(+), 2 deletions(-) diff --git a/src/cpp/fastdds/subscriber/DataReaderImpl.cpp b/src/cpp/fastdds/subscriber/DataReaderImpl.cpp index 5bcd486ca25..9e2d744c25c 100644 --- a/src/cpp/fastdds/subscriber/DataReaderImpl.cpp +++ b/src/cpp/fastdds/subscriber/DataReaderImpl.cpp @@ -841,10 +841,10 @@ void DataReaderImpl::InnerDataReaderListener::on_requested_incompatible_qos( bool DataReaderImpl::on_new_cache_change_added( const CacheChange_t* const change) { + std::lock_guard guard(reader_->getMutex()); + if (qos_.deadline().period != c_TimeInfinite) { - std::unique_lock lock(reader_->getMutex()); - if (!history_.set_next_deadline( change->instanceHandle, steady_clock::now() + duration_cast(deadline_duration_us_))) @@ -861,6 +861,8 @@ bool DataReaderImpl::on_new_cache_change_added( } } + history_.update_instance_nts(change); + CacheChange_t* new_change = const_cast(change); if (qos_.lifespan().duration == c_TimeInfinite) diff --git a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp index 926205d4fde..9e79fb46a1c 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp @@ -532,6 +532,16 @@ ReaderHistory::iterator DataReaderHistory::remove_change_nts( return ReaderHistory::remove_change_nts(removal, release); } +void DataReaderHistory::update_instance_nts( + const CacheChange_t* const change) +{ + InstanceCollection::iterator vit; + vit = keyed_changes_.find(change->instanceHandle); + + assert(vit != keyed_changes_.end()); + vit->second.update_state(change->kind, change->writerGUID); +} + } // namespace detail } // namsepace dds } // namespace fastdds diff --git a/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp b/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp index 225c5e9140a..d2e0ba01789 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp @@ -160,6 +160,9 @@ class DataReaderHistory : public eprosima::fastrtps::rtps::ReaderHistory const InstanceHandle_t& handle, bool exact); + void update_instance_nts( + const CacheChange_t* const change); + private: using InstanceCollection = std::map; diff --git a/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp b/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp index 76d218a163a..9d781e10483 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp @@ -51,6 +51,63 @@ struct DataReaderInstance int32_t disposed_generation_count = 0; //! Current no_writers generation of the instance int32_t no_writers_generation_count = 0; + + bool update_state( + const fastrtps::rtps::ChangeKind_t change_kind, + const fastrtps::rtps::GUID_t& writer_guid, + const uint32_t ownership_strength = 0) + { + bool ret_val = false; + + switch (change_kind) + { + case fastrtps::rtps::ALIVE: + ret_val = writer_alive(writer_guid, ownership_strength); + break; + + case fastrtps::rtps::NOT_ALIVE_DISPOSED: + ret_val = writer_dispose(writer_guid, ownership_strength); + break; + + case fastrtps::rtps::NOT_ALIVE_DISPOSED_UNREGISTERED: + ret_val = writer_dispose(writer_guid, ownership_strength); + ret_val |= writer_unregister(writer_guid); + break; + + case fastrtps::rtps::NOT_ALIVE_UNREGISTERED: + ret_val = writer_unregister(writer_guid); + break; + + default: + // TODO (Miguel C): log error / assert + break; + } + + return ret_val; + } + +private: + + bool writer_alive( + const fastrtps::rtps::GUID_t& /*writer_guid*/, + const uint32_t /*ownership_strength*/) + { + return false; + } + + bool writer_dispose( + const fastrtps::rtps::GUID_t& /*writer_guid*/, + const uint32_t /*ownership_strength*/) + { + return false; + } + + bool writer_unregister( + const fastrtps::rtps::GUID_t& /*writer_guid*/) + { + return false; + } + }; } /* namespace detail */ From b5556172a581a4d85a55d8a641c3912f4c0f1a1e Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Tue, 26 Oct 2021 15:53:59 +0200 Subject: [PATCH 15/56] Refs 12400. Adding alive_writers and current_owner to DataReaderInstance. Signed-off-by: Miguel Company --- src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp b/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp index 9d781e10483..0826441c62f 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp @@ -21,6 +21,7 @@ #include #include +#include #include #include @@ -38,9 +39,14 @@ namespace detail { struct DataReaderInstance { using ChangeCollection = eprosima::fastrtps::ResourceLimitedVector; + using WriterCollection = std::map; //! A vector of DataReader changes belonging to the same instance ChangeCollection cache_changes; + //! The list of alive writers for this instance + WriterCollection alive_writers; + //! GUID and strength of the current maximum strength writer + std::pair current_owner{ {}, 0 }; //! The time when the group will miss the deadline std::chrono::steady_clock::time_point next_deadline_us; //! Current view state of the instance From 0ae7cd809c069ce6d5bb85046087a2b7032702b0 Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Tue, 26 Oct 2021 15:59:11 +0200 Subject: [PATCH 16/56] Refs 12400. Implementing writer_alive. Signed-off-by: Miguel Company --- .../subscriber/history/DataReaderInstance.hpp | 29 +++++++++++++++++-- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp b/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp index 0826441c62f..1400a81e595 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp @@ -95,10 +95,33 @@ struct DataReaderInstance private: bool writer_alive( - const fastrtps::rtps::GUID_t& /*writer_guid*/, - const uint32_t /*ownership_strength*/) + const fastrtps::rtps::GUID_t& writer_guid, + const uint32_t ownership_strength) { - return false; + bool ret_val = false; + + alive_writers[writer_guid] = ownership_strength; + + if (ownership_strength >= current_owner.second) + { + current_owner.first = writer_guid; + current_owner.second = ownership_strength; + + if (InstanceStateKind::NOT_ALIVE_DISPOSED_INSTANCE_STATE == instance_state) + { + ret_val = true; + ++disposed_generation_count; + } + else if (InstanceStateKind::NOT_ALIVE_NO_WRITERS_INSTANCE_STATE == instance_state) + { + ret_val = true; + ++no_writers_generation_count; + } + + instance_state = InstanceStateKind::ALIVE_INSTANCE_STATE; + } + + return ret_val; } bool writer_dispose( From 1284cbcf7d05777d2583d564934800be430f4ba2 Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Tue, 26 Oct 2021 16:05:21 +0200 Subject: [PATCH 17/56] Refs 12400. Implementing writer_dispose. Signed-off-by: Miguel Company --- .../subscriber/history/DataReaderInstance.hpp | 21 ++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp b/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp index 1400a81e595..784120a122b 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp @@ -125,10 +125,25 @@ struct DataReaderInstance } bool writer_dispose( - const fastrtps::rtps::GUID_t& /*writer_guid*/, - const uint32_t /*ownership_strength*/) + const fastrtps::rtps::GUID_t& writer_guid, + const uint32_t ownership_strength) { - return false; + bool ret_val = false; + + alive_writers[writer_guid] = ownership_strength; + if (ownership_strength >= current_owner.second) + { + current_owner.first = writer_guid; + current_owner.second = ownership_strength; + + if (InstanceStateKind::ALIVE_INSTANCE_STATE == instance_state) + { + ret_val = true; + instance_state = InstanceStateKind::NOT_ALIVE_DISPOSED_INSTANCE_STATE; + } + } + + return ret_val; } bool writer_unregister( From d86399100726fcccd27e4bf23c6c9b8963133006 Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Tue, 26 Oct 2021 16:55:45 +0200 Subject: [PATCH 18/56] Refs 12400. Implementing writer_unregister. Signed-off-by: Miguel Company --- .../subscriber/history/DataReaderInstance.hpp | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp b/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp index 784120a122b..25345b9fa76 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp @@ -147,9 +147,25 @@ struct DataReaderInstance } bool writer_unregister( - const fastrtps::rtps::GUID_t& /*writer_guid*/) + const fastrtps::rtps::GUID_t& writer_guid) { - return false; + bool ret_val = false; + + alive_writers.erase(writer_guid); + + if (writer_guid == current_owner.first) + { + current_owner.second = 0; + current_owner.first = fastrtps::rtps::c_Guid_Unknown; + } + + if (alive_writers.empty() && (InstanceStateKind::ALIVE_INSTANCE_STATE == instance_state)) + { + ret_val = true; + instance_state = InstanceStateKind::NOT_ALIVE_NO_WRITERS_INSTANCE_STATE; + } + + return ret_val; } }; From 7f6f641fcb7f66c2f72267f4e86c0806bfea3c8e Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Tue, 26 Oct 2021 16:57:28 +0200 Subject: [PATCH 19/56] Refs 12400. Setting NOT_NEW on returned instances. Signed-off-by: Miguel Company --- src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp b/src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp index d86d4207f1d..a03f7869e47 100644 --- a/src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp +++ b/src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp @@ -180,6 +180,7 @@ struct ReadTakeCommand if (current_slot_ > first_slot) { + instance_.second->view_state = ViewStateKind::NOT_NEW_VIEW_STATE; ret_val = true; // complete sample infos From 664aca14759232b24bfb8280b0368bd05701ee77 Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Wed, 27 Oct 2021 07:18:26 +0200 Subject: [PATCH 20/56] Refs 12400. Correct return code when returning samples with no data. Signed-off-by: Miguel Company --- .../fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp b/src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp index a03f7869e47..fb44f899d3b 100644 --- a/src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp +++ b/src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp @@ -329,11 +329,10 @@ struct ReadTakeCommand deserialization_error = true; return false; } - - // Mark that some data is available - return_value_ = ReturnCode_t::RETCODE_OK; } + // Mark that some data is available + return_value_ = ReturnCode_t::RETCODE_OK; ++current_slot_; --remaining_samples_; ret_val = true; From 570b43fb84a6ecf5600f58097dd0dbb4b1cf2e2d Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Wed, 27 Oct 2021 07:20:23 +0200 Subject: [PATCH 21/56] Refs 12400. Set view_state to NEW when changing instance_state to ALIVE. Signed-off-by: Miguel Company --- src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp b/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp index 25345b9fa76..5f7fe77c214 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp @@ -109,13 +109,15 @@ struct DataReaderInstance if (InstanceStateKind::NOT_ALIVE_DISPOSED_INSTANCE_STATE == instance_state) { - ret_val = true; ++disposed_generation_count; + view_state = ViewStateKind::NEW_VIEW_STATE; + ret_val = true; } else if (InstanceStateKind::NOT_ALIVE_NO_WRITERS_INSTANCE_STATE == instance_state) { - ret_val = true; ++no_writers_generation_count; + view_state = ViewStateKind::NEW_VIEW_STATE; + ret_val = true; } instance_state = InstanceStateKind::ALIVE_INSTANCE_STATE; From 3020516dbbfa75e2cad552f200a9df8f0c7e2106 Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Wed, 27 Oct 2021 09:32:29 +0200 Subject: [PATCH 22/56] Refs 12400. Moving generation counts into CacheChange_t. Signed-off-by: Miguel Company --- include/fastdds/rtps/common/CacheChange.h | 4 +++ .../DataReaderImpl/ReadTakeCommand.hpp | 26 +++++++++---------- .../history/DataReaderCacheChange.hpp | 10 +------ .../subscriber/history/DataReaderHistory.cpp | 19 +++++++------- 4 files changed, 27 insertions(+), 32 deletions(-) diff --git a/include/fastdds/rtps/common/CacheChange.h b/include/fastdds/rtps/common/CacheChange.h index 556149ad8a9..1f3c260ca8a 100644 --- a/include/fastdds/rtps/common/CacheChange.h +++ b/include/fastdds/rtps/common/CacheChange.h @@ -57,6 +57,10 @@ struct CacheChangeReaderInfo_t { //!Reception TimeStamp (only used in Readers) Time_t receptionTimestamp; + //! Disposed generation of the instance when this entry was added to it + int32_t disposed_generation_count; + //! No-writers generation of the instance when this entry was added to it + int32_t no_writers_generation_count; }; /** diff --git a/src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp b/src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp index fb44f899d3b..fbf25352305 100644 --- a/src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp +++ b/src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp @@ -111,7 +111,7 @@ struct ReadTakeCommand auto it = instance_.second->cache_changes.begin(); while (!finished_ && it != instance_.second->cache_changes.end()) { - CacheChange_t* change = it->change; + CacheChange_t* change = *it; SampleStateKind check; check = change->isRead ? SampleStateKind::READ_SAMPLE_STATE : SampleStateKind::NOT_READ_SAMPLE_STATE; if ((check & states_.sample_states) != 0) @@ -213,24 +213,24 @@ struct ReadTakeCommand const DataReaderInstance& instance, const DataReaderCacheChange& item) { - info.sample_state = item.change->isRead ? READ_SAMPLE_STATE : NOT_READ_SAMPLE_STATE; + info.sample_state = item->isRead ? READ_SAMPLE_STATE : NOT_READ_SAMPLE_STATE; info.instance_state = instance.instance_state; info.view_state = instance.view_state; - info.disposed_generation_count = item.disposed_generation_count; - info.no_writers_generation_count = item.no_writers_generation_count; + info.disposed_generation_count = item->reader_info.disposed_generation_count; + info.no_writers_generation_count = item->reader_info.no_writers_generation_count; info.sample_rank = 0; info.generation_rank = 0; info.absoulte_generation_rank = 0; - info.source_timestamp = item.change->sourceTimestamp; - info.reception_timestamp = item.change->reader_info.receptionTimestamp; - info.instance_handle = item.change->instanceHandle; - info.publication_handle = InstanceHandle_t(item.change->writerGUID); - info.sample_identity.writer_guid(item.change->writerGUID); - info.sample_identity.sequence_number(item.change->sequenceNumber); - info.related_sample_identity = item.change->write_params.sample_identity(); + info.source_timestamp = item->sourceTimestamp; + info.reception_timestamp = item->reader_info.receptionTimestamp; + info.instance_handle = item->instanceHandle; + info.publication_handle = InstanceHandle_t(item->writerGUID); + info.sample_identity.writer_guid(item->writerGUID); + info.sample_identity.sequence_number(item->sequenceNumber); + info.related_sample_identity = item->write_params.sample_identity(); info.valid_data = true; - switch (item.change->kind) + switch (item->kind) { case eprosima::fastrtps::rtps::NOT_ALIVE_DISPOSED: case eprosima::fastrtps::rtps::NOT_ALIVE_DISPOSED_UNREGISTERED: @@ -321,7 +321,7 @@ struct ReadTakeCommand generate_info(item); if (sample_infos_[current_slot_].valid_data) { - if (!deserialize_sample(item.change)) + if (!deserialize_sample(item)) { // Decrement length of collections data_values_.length(current_slot_); diff --git a/src/cpp/fastdds/subscriber/history/DataReaderCacheChange.hpp b/src/cpp/fastdds/subscriber/history/DataReaderCacheChange.hpp index 8ef7a9a2fd8..5b13109f721 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderCacheChange.hpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderCacheChange.hpp @@ -29,15 +29,7 @@ namespace dds { namespace detail { /// A DataReader cache entry -struct DataReaderCacheChange -{ - //! Low level cache entry - fastrtps::rtps::CacheChange_t* change; - //! Disposed generation of the instance when this entry was added to it - int32_t disposed_generation_count; - //! No-writers generation of the instance when this entry was added to it - int32_t no_writers_generation_count; -}; +using DataReaderCacheChange = fastrtps::rtps::CacheChange_t*; } /* namespace detail */ } /* namespace dds */ diff --git a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp index 9e79fb46a1c..2ad5713ced0 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp @@ -203,7 +203,7 @@ bool DataReaderHistory::received_change_keep_last( else { // Try to substitute the oldest sample. - CacheChange_t* first_change = instance_changes.at(0).change; + CacheChange_t* first_change = instance_changes.at(0); if (a_change->sourceTimestamp < first_change->sourceTimestamp) { // Received change is older than oldest, and should be discarded @@ -248,12 +248,11 @@ bool DataReaderHistory::add_received_change_with_key( } //ADD TO KEY VECTOR - DataReaderCacheChange item{ a_change, instance.disposed_generation_count, - instance.no_writers_generation_count }; + DataReaderCacheChange item = a_change; eprosima::utilities::collections::sorted_vector_insert(instance.cache_changes, item, [](const DataReaderCacheChange& lhs, const DataReaderCacheChange& rhs) { - return lhs.change->sourceTimestamp < rhs.change->sourceTimestamp; + return lhs->sourceTimestamp < rhs->sourceTimestamp; }); logInfo(SUBSCRIBER, mp_reader->getGuid().entityId @@ -289,7 +288,7 @@ bool DataReaderHistory::get_first_untaken_info( std::find_if(instance_changes.cbegin(), instance_changes.cend(), [change](const DataReaderCacheChange& v) { - return v.change == change; + return v == change; }); ReadTakeCommand::generate_info(info, it->second, *item); mp_reader->change_read_by_user(change, wp, false); @@ -347,8 +346,8 @@ bool DataReaderHistory::remove_change_sub( { for (auto chit = vit->second.cache_changes.begin(); chit != vit->second.cache_changes.end(); ++chit) { - if (chit->change->sequenceNumber == change->sequenceNumber && - chit->change->writerGUID == change->writerGUID) + if ((*chit)->sequenceNumber == change->sequenceNumber && + (*chit)->writerGUID == change->writerGUID) { vit->second.cache_changes.erase(chit); found = true; @@ -387,8 +386,8 @@ bool DataReaderHistory::remove_change_sub( { for (auto chit = vit->second.cache_changes.begin(); chit != vit->second.cache_changes.end(); ++chit) { - if (chit->change->sequenceNumber == change->sequenceNumber && - chit->change->writerGUID == change->writerGUID) + if ((*chit)->sequenceNumber == change->sequenceNumber && + (*chit)->writerGUID == change->writerGUID) { assert(it == chit); it = vit->second.cache_changes.erase(chit); @@ -524,7 +523,7 @@ ReaderHistory::iterator DataReaderHistory::remove_change_nts( auto& c = it->second.cache_changes; c.erase(std::remove_if(c.begin(), c.end(), [p_sample](DataReaderCacheChange& elem) { - return elem.change == p_sample; + return elem == p_sample; }), c.end()); } From 48f7a44bea5c021e9203f0dfc0052043975db5bc Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Wed, 27 Oct 2021 09:37:18 +0200 Subject: [PATCH 23/56] Refs 12400. Assigning generation counts after processing instance state. Signed-off-by: Miguel Company --- src/cpp/fastdds/subscriber/DataReaderImpl.cpp | 3 +-- src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp | 4 +++- src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/cpp/fastdds/subscriber/DataReaderImpl.cpp b/src/cpp/fastdds/subscriber/DataReaderImpl.cpp index 9e2d744c25c..428173c01cd 100644 --- a/src/cpp/fastdds/subscriber/DataReaderImpl.cpp +++ b/src/cpp/fastdds/subscriber/DataReaderImpl.cpp @@ -861,9 +861,8 @@ bool DataReaderImpl::on_new_cache_change_added( } } - history_.update_instance_nts(change); - CacheChange_t* new_change = const_cast(change); + history_.update_instance_nts(new_change); if (qos_.lifespan().duration == c_TimeInfinite) { diff --git a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp index 2ad5713ced0..9bbc03723af 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp @@ -532,13 +532,15 @@ ReaderHistory::iterator DataReaderHistory::remove_change_nts( } void DataReaderHistory::update_instance_nts( - const CacheChange_t* const change) + CacheChange_t* const change) { InstanceCollection::iterator vit; vit = keyed_changes_.find(change->instanceHandle); assert(vit != keyed_changes_.end()); vit->second.update_state(change->kind, change->writerGUID); + change->reader_info.disposed_generation_count = vit->second.disposed_generation_count; + change->reader_info.no_writers_generation_count = vit->second.no_writers_generation_count; } } // namespace detail diff --git a/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp b/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp index d2e0ba01789..597a7b77518 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp @@ -161,7 +161,7 @@ class DataReaderHistory : public eprosima::fastrtps::rtps::ReaderHistory bool exact); void update_instance_nts( - const CacheChange_t* const change); + CacheChange_t* const change); private: From 3ee50bd407368d5e7576fd9cffe8a3863143ebd2 Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Wed, 27 Oct 2021 09:39:11 +0200 Subject: [PATCH 24/56] Refs 12400. Update instance_state when writer becomes not alive. Signed-off-by: Miguel Company --- src/cpp/fastdds/subscriber/DataReaderImpl.cpp | 10 ++++++++++ .../subscriber/history/DataReaderHistory.cpp | 15 +++++++++++++++ .../subscriber/history/DataReaderHistory.hpp | 7 +++++++ .../subscriber/history/DataReaderInstance.hpp | 6 ++++++ 4 files changed, 38 insertions(+) diff --git a/src/cpp/fastdds/subscriber/DataReaderImpl.cpp b/src/cpp/fastdds/subscriber/DataReaderImpl.cpp index 428173c01cd..a8029cb0f57 100644 --- a/src/cpp/fastdds/subscriber/DataReaderImpl.cpp +++ b/src/cpp/fastdds/subscriber/DataReaderImpl.cpp @@ -918,6 +918,11 @@ void DataReaderImpl::update_subscription_matched_status( subscription_matched_status_.last_publication_handle = status.last_publication_handle; } + if (count_change < 0) + { + history_.writer_unmatched(iHandle2GUID(status.last_publication_handle)); + } + StatusMask notify_status = StatusMask::subscription_matched(); DataReaderListener* listener = get_listener_for(notify_status); if (listener != nullptr) @@ -1185,6 +1190,11 @@ RequestedIncompatibleQosStatus& DataReaderImpl::update_requested_incompatible_qo LivelinessChangedStatus& DataReaderImpl::update_liveliness_status( const fastrtps::LivelinessChangedStatus& status) { + if (0 < status.not_alive_count_change) + { + history_.writer_liveliness_lost(iHandle2GUID(status.last_publication_handle)); + } + liveliness_changed_status_.alive_count = status.alive_count; liveliness_changed_status_.not_alive_count = status.not_alive_count; liveliness_changed_status_.alive_count_change += status.alive_count_change; diff --git a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp index 9bbc03723af..3ca926d7a25 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp @@ -543,6 +543,21 @@ void DataReaderHistory::update_instance_nts( change->reader_info.no_writers_generation_count = vit->second.no_writers_generation_count; } +void DataReaderHistory::writer_liveliness_lost( + const fastrtps::rtps::GUID_t& writer_guid) +{ + for (auto& it : keyed_changes_) + { + it.second.writer_removed(writer_guid); + } +} + +void DataReaderHistory::writer_unmatched( + const fastrtps::rtps::GUID_t& writer_guid) +{ + writer_liveliness_lost(writer_guid); +} + } // namespace detail } // namsepace dds } // namespace fastdds diff --git a/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp b/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp index 597a7b77518..f0e619c8296 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp @@ -34,6 +34,7 @@ #include #include +#include #include #include #include @@ -163,6 +164,12 @@ class DataReaderHistory : public eprosima::fastrtps::rtps::ReaderHistory void update_instance_nts( CacheChange_t* const change); + void writer_liveliness_lost( + const fastrtps::rtps::GUID_t& writer_guid); + + void writer_unmatched( + const fastrtps::rtps::GUID_t& writer_guid); + private: using InstanceCollection = std::map; diff --git a/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp b/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp index 5f7fe77c214..b4557e2e478 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp @@ -92,6 +92,12 @@ struct DataReaderInstance return ret_val; } + bool writer_removed( + const fastrtps::rtps::GUID_t& writer_guid) + { + return writer_unregister(writer_guid); + } + private: bool writer_alive( From 50b88687410f58ada35567df3c993770e5a10dd0 Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Wed, 27 Oct 2021 12:19:37 +0200 Subject: [PATCH 25/56] Refs 12400. Clear alive_writers when changing generation. Signed-off-by: Miguel Company --- src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp b/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp index b4557e2e478..52b556397d4 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp @@ -106,8 +106,6 @@ struct DataReaderInstance { bool ret_val = false; - alive_writers[writer_guid] = ownership_strength; - if (ownership_strength >= current_owner.second) { current_owner.first = writer_guid; @@ -116,16 +114,19 @@ struct DataReaderInstance if (InstanceStateKind::NOT_ALIVE_DISPOSED_INSTANCE_STATE == instance_state) { ++disposed_generation_count; + alive_writers.clear(); view_state = ViewStateKind::NEW_VIEW_STATE; ret_val = true; } else if (InstanceStateKind::NOT_ALIVE_NO_WRITERS_INSTANCE_STATE == instance_state) { ++no_writers_generation_count; + alive_writers.clear(); view_state = ViewStateKind::NEW_VIEW_STATE; ret_val = true; } + alive_writers[writer_guid] = ownership_strength; instance_state = InstanceStateKind::ALIVE_INSTANCE_STATE; } From 2f61a2e77c7e31d473701a76093aab6ed8c12feb Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Wed, 27 Oct 2021 12:28:41 +0200 Subject: [PATCH 26/56] Refs 12400. Add writer_unmatched to ReaderHistory. Signed-off-by: Miguel Company --- include/fastdds/rtps/history/ReaderHistory.h | 6 ++++++ src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp | 3 ++- src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp | 4 ++-- src/cpp/rtps/reader/StatefulReader.cpp | 2 +- src/cpp/rtps/reader/StatelessReader.cpp | 2 +- .../rtps/ReaderHistory/fastdds/rtps/history/ReaderHistory.h | 6 ++++++ 6 files changed, 18 insertions(+), 5 deletions(-) diff --git a/include/fastdds/rtps/history/ReaderHistory.h b/include/fastdds/rtps/history/ReaderHistory.h index 2d83ca02bbd..b3ca936cf82 100644 --- a/include/fastdds/rtps/history/ReaderHistory.h +++ b/include/fastdds/rtps/history/ReaderHistory.h @@ -152,6 +152,12 @@ class ReaderHistory : public History CacheChange_t** min_change, const GUID_t& writerGuid); + RTPS_DllAPI virtual bool writer_unmatched( + const GUID_t& writer_guid) + { + return remove_changes_with_guid(writer_guid); + } + protected: RTPS_DllAPI bool do_reserve_cache( diff --git a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp index 3ca926d7a25..7ac7757455d 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp @@ -552,10 +552,11 @@ void DataReaderHistory::writer_liveliness_lost( } } -void DataReaderHistory::writer_unmatched( +bool DataReaderHistory::writer_unmatched( const fastrtps::rtps::GUID_t& writer_guid) { writer_liveliness_lost(writer_guid); + return ReaderHistory::writer_unmatched(writer_guid); } } // namespace detail diff --git a/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp b/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp index f0e619c8296..33b54ce67ba 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp @@ -167,8 +167,8 @@ class DataReaderHistory : public eprosima::fastrtps::rtps::ReaderHistory void writer_liveliness_lost( const fastrtps::rtps::GUID_t& writer_guid); - void writer_unmatched( - const fastrtps::rtps::GUID_t& writer_guid); + bool writer_unmatched( + const fastrtps::rtps::GUID_t& writer_guid) override; private: diff --git a/src/cpp/rtps/reader/StatefulReader.cpp b/src/cpp/rtps/reader/StatefulReader.cpp index 5d08dd09b66..c188c2f576c 100644 --- a/src/cpp/rtps/reader/StatefulReader.cpp +++ b/src/cpp/rtps/reader/StatefulReader.cpp @@ -285,7 +285,7 @@ bool StatefulReader::matched_writer_remove( if (is_alive_) { //Remove cachechanges belonging to the unmatched writer - mp_history->remove_changes_with_guid(writer_guid); + mp_history->writer_unmatched(writer_guid); if (liveliness_lease_duration_ < c_TimeInfinite) { diff --git a/src/cpp/rtps/reader/StatelessReader.cpp b/src/cpp/rtps/reader/StatelessReader.cpp index 738ba165d83..19e591c1eb1 100644 --- a/src/cpp/rtps/reader/StatelessReader.cpp +++ b/src/cpp/rtps/reader/StatelessReader.cpp @@ -178,7 +178,7 @@ bool StatelessReader::matched_writer_remove( std::lock_guard guard(mp_mutex); //Remove cachechanges belonging to the unmatched writer - mp_history->remove_changes_with_guid(writer_guid); + mp_history->writer_unmatched(writer_guid); if (liveliness_lease_duration_ < c_TimeInfinite) { diff --git a/test/mock/rtps/ReaderHistory/fastdds/rtps/history/ReaderHistory.h b/test/mock/rtps/ReaderHistory/fastdds/rtps/history/ReaderHistory.h index 79225ff3fad..09e7ef4061f 100644 --- a/test/mock/rtps/ReaderHistory/fastdds/rtps/history/ReaderHistory.h +++ b/test/mock/rtps/ReaderHistory/fastdds/rtps/history/ReaderHistory.h @@ -131,6 +131,12 @@ class ReaderHistory return m_changes.erase(removal); } + virtual bool writer_unmatched( + const GUID_t& /*writer_guid*/) + { + return true; + } + HistoryAttributes m_att; protected: From 5e57866df6584b1b5a500e55873fb22638689bfe Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Thu, 28 Oct 2021 11:03:51 +0200 Subject: [PATCH 27/56] Refs 12400. NOT_ALIVE_UNREGISTERED should not return valid data. Signed-off-by: Miguel Company --- src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp b/src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp index fbf25352305..cc02eed036a 100644 --- a/src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp +++ b/src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp @@ -234,6 +234,7 @@ struct ReadTakeCommand { case eprosima::fastrtps::rtps::NOT_ALIVE_DISPOSED: case eprosima::fastrtps::rtps::NOT_ALIVE_DISPOSED_UNREGISTERED: + case eprosima::fastrtps::rtps::NOT_ALIVE_UNREGISTERED: info.valid_data = false; break; case eprosima::fastrtps::rtps::ALIVE: From d3d0e3d3f2a5127d56d07bc4874cd7f5352879c2 Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Thu, 28 Oct 2021 15:57:41 +0200 Subject: [PATCH 28/56] Refs 12400. Set autodispose_unregistered_instances to false on test. Signed-off-by: Miguel Company --- test/unittest/dds/subscriber/DataReaderTests.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/test/unittest/dds/subscriber/DataReaderTests.cpp b/test/unittest/dds/subscriber/DataReaderTests.cpp index 384839bb9f5..0edc6f06e3e 100644 --- a/test/unittest/dds/subscriber/DataReaderTests.cpp +++ b/test/unittest/dds/subscriber/DataReaderTests.cpp @@ -1527,6 +1527,7 @@ TEST_F(DataReaderTests, sample_info) writer_qos_.resource_limits().max_instances = 2; writer_qos_.resource_limits().max_samples_per_instance = 1; writer_qos_.resource_limits().max_samples = 2; + writer_qos_.writer_data_lifecycle().autodispose_unregistered_instances = false; data_[0].index(1); data_[1].index(2); From 5b4c855a97c75cfdec065c0aa9896f6c2a574ebb Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Thu, 28 Oct 2021 16:12:21 +0200 Subject: [PATCH 29/56] Refs 12400. Remove instance when it becomes empty and is not alive. Signed-off-by: Miguel Company --- .../subscriber/DataReaderImpl/ReadTakeCommand.hpp | 1 + .../subscriber/history/DataReaderHistory.cpp | 13 +++++++++++++ .../subscriber/history/DataReaderHistory.hpp | 3 +++ 3 files changed, 17 insertions(+) diff --git a/src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp b/src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp index cc02eed036a..66af2a2db7c 100644 --- a/src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp +++ b/src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp @@ -286,6 +286,7 @@ struct ReadTakeCommand bool next_instance() { + history_.check_and_remove_instance(instance_); if (single_instance_) { finished_ = true; diff --git a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp index 7ac7757455d..fe6e3291357 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp @@ -506,6 +506,19 @@ std::pair DataReaderHistory::lookup_inst return { false, {InstanceHandle_t(), nullptr} }; } +void DataReaderHistory::check_and_remove_instance( + DataReaderHistory::instance_info& instance_info) +{ + DataReaderInstance* instance = instance_info.second; + if (instance->cache_changes.empty() && + (InstanceStateKind::ALIVE_INSTANCE_STATE != instance->instance_state) && + instance_info.first.isDefined()) + { + keyed_changes_.erase(instance_info.first); + instance_info.second = nullptr; + } +} + ReaderHistory::iterator DataReaderHistory::remove_change_nts( ReaderHistory::const_iterator removal, bool release) diff --git a/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp b/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp index 33b54ce67ba..26a020789b4 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp @@ -170,6 +170,9 @@ class DataReaderHistory : public eprosima::fastrtps::rtps::ReaderHistory bool writer_unmatched( const fastrtps::rtps::GUID_t& writer_guid) override; + void check_and_remove_instance( + instance_info& instance_info); + private: using InstanceCollection = std::map; From 2a9f55eccb5b458231b38c60312badcfaf73d3cc Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Fri, 29 Oct 2021 07:28:41 +0200 Subject: [PATCH 30/56] Refs 12400. Refactor into writer_not_alive. Signed-off-by: Miguel Company --- src/cpp/fastdds/subscriber/DataReaderImpl.cpp | 4 ++-- src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp | 3 +-- src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp | 2 +- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/cpp/fastdds/subscriber/DataReaderImpl.cpp b/src/cpp/fastdds/subscriber/DataReaderImpl.cpp index a8029cb0f57..2444a17553e 100644 --- a/src/cpp/fastdds/subscriber/DataReaderImpl.cpp +++ b/src/cpp/fastdds/subscriber/DataReaderImpl.cpp @@ -920,7 +920,7 @@ void DataReaderImpl::update_subscription_matched_status( if (count_change < 0) { - history_.writer_unmatched(iHandle2GUID(status.last_publication_handle)); + history_.writer_not_alive(iHandle2GUID(status.last_publication_handle)); } StatusMask notify_status = StatusMask::subscription_matched(); @@ -1192,7 +1192,7 @@ LivelinessChangedStatus& DataReaderImpl::update_liveliness_status( { if (0 < status.not_alive_count_change) { - history_.writer_liveliness_lost(iHandle2GUID(status.last_publication_handle)); + history_.writer_not_alive(iHandle2GUID(status.last_publication_handle)); } liveliness_changed_status_.alive_count = status.alive_count; diff --git a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp index fe6e3291357..7a181e9cc9d 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp @@ -556,7 +556,7 @@ void DataReaderHistory::update_instance_nts( change->reader_info.no_writers_generation_count = vit->second.no_writers_generation_count; } -void DataReaderHistory::writer_liveliness_lost( +void DataReaderHistory::writer_not_alive( const fastrtps::rtps::GUID_t& writer_guid) { for (auto& it : keyed_changes_) @@ -568,7 +568,6 @@ void DataReaderHistory::writer_liveliness_lost( bool DataReaderHistory::writer_unmatched( const fastrtps::rtps::GUID_t& writer_guid) { - writer_liveliness_lost(writer_guid); return ReaderHistory::writer_unmatched(writer_guid); } diff --git a/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp b/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp index 26a020789b4..a763108c590 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp @@ -164,7 +164,7 @@ class DataReaderHistory : public eprosima::fastrtps::rtps::ReaderHistory void update_instance_nts( CacheChange_t* const change); - void writer_liveliness_lost( + void writer_not_alive( const fastrtps::rtps::GUID_t& writer_guid); bool writer_unmatched( From 38ca299d93a4b3e6feb22fe6e6f3d408193c2426 Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Fri, 29 Oct 2021 08:05:13 +0200 Subject: [PATCH 31/56] Refs 12400. Keeping samples from unmatched writers. Signed-off-by: Miguel Company --- .../subscriber/history/DataReaderHistory.cpp | 3 +- src/cpp/rtps/reader/StatefulReader.cpp | 49 +++++++------------ 2 files changed, 20 insertions(+), 32 deletions(-) diff --git a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp index 7a181e9cc9d..11d80e644ef 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp @@ -568,7 +568,8 @@ void DataReaderHistory::writer_not_alive( bool DataReaderHistory::writer_unmatched( const fastrtps::rtps::GUID_t& writer_guid) { - return ReaderHistory::writer_unmatched(writer_guid); + static_cast(writer_guid); + return true; } } // namespace detail diff --git a/src/cpp/rtps/reader/StatefulReader.cpp b/src/cpp/rtps/reader/StatefulReader.cpp index c188c2f576c..cf43a0e5c89 100644 --- a/src/cpp/rtps/reader/StatefulReader.cpp +++ b/src/cpp/rtps/reader/StatefulReader.cpp @@ -793,30 +793,20 @@ bool StatefulReader::change_removed_by_history( { if (wp != nullptr || matched_writer_lookup(a_change->writerGUID, &wp)) { - if (a_change->is_fully_assembled()) - { - if (!a_change->isRead && wp->available_changes_max() >= a_change->sequenceNumber) - { - if (0 < total_unread_) - { - --total_unread_; - } - } - - - wp->change_removed_from_history(a_change->sequenceNumber); - } - return true; + wp->change_removed_from_history(a_change->sequenceNumber); } - else + + if (a_change->is_fully_assembled() && + !a_change->isRead && + get_last_notified(a_change->writerGUID) >= a_change->sequenceNumber) { - if (a_change->writerGUID.entityId != m_trustedWriterEntityId) + if (0 < total_unread_) { - // trusted entities messages mean no havoc - logError(RTPS_READER, - " You should always find the WP associated with a change, something is very wrong"); + --total_unread_; } + } + return true; } //Simulate a datasharing notification to process any pending payloads that were waiting due to full history @@ -1143,16 +1133,14 @@ bool StatefulReader::begin_sample_access_nts( const GUID_t& writer_guid = change->writerGUID; is_future_change = false; - if (!matched_writer_lookup(writer_guid, &wp)) - { - return false; - } - - SequenceNumber_t seq; - seq = wp->available_changes_max(); - if (seq < change->sequenceNumber) + if (matched_writer_lookup(writer_guid, &wp)) { - is_future_change = true; + SequenceNumber_t seq; + seq = wp->available_changes_max(); + if (seq < change->sequenceNumber) + { + is_future_change = true; + } } return true; @@ -1171,8 +1159,7 @@ void StatefulReader::change_read_by_user( WriterProxy* writer, bool mark_as_read) { - assert(writer != nullptr); - assert(change->writerGUID == writer->guid()); + assert(!writer || change->writerGUID == writer->guid()); // Mark change as read if (mark_as_read && !change->isRead) @@ -1185,7 +1172,7 @@ void StatefulReader::change_read_by_user( } // If not datasharing, we are done - if (!writer->is_datasharing_writer()) + if (!writer || !writer->is_datasharing_writer()) { return; } From 961408c4a5f8f236b3fb5947b860db36406be858 Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Fri, 29 Oct 2021 12:08:46 +0200 Subject: [PATCH 32/56] Refs 12400. Avoid keeping non-notified samples. Signed-off-by: Miguel Company --- include/fastdds/rtps/history/ReaderHistory.h | 42 ++++++++++++++++++- .../subscriber/history/DataReaderHistory.cpp | 7 ---- .../subscriber/history/DataReaderHistory.hpp | 3 -- src/cpp/rtps/history/ReaderHistory.cpp | 29 ++++--------- src/cpp/rtps/reader/StatefulReader.cpp | 2 +- src/cpp/rtps/reader/StatelessReader.cpp | 2 +- 6 files changed, 50 insertions(+), 35 deletions(-) diff --git a/include/fastdds/rtps/history/ReaderHistory.h b/include/fastdds/rtps/history/ReaderHistory.h index b3ca936cf82..3341739c991 100644 --- a/include/fastdds/rtps/history/ReaderHistory.h +++ b/include/fastdds/rtps/history/ReaderHistory.h @@ -153,9 +153,15 @@ class ReaderHistory : public History const GUID_t& writerGuid); RTPS_DllAPI virtual bool writer_unmatched( - const GUID_t& writer_guid) + const GUID_t& writer_guid, + const SequenceNumber_t& last_notified_seq) { - return remove_changes_with_guid(writer_guid); + return remove_changes_with_pred( + [writer_guid, last_notified_seq](CacheChange_t* ch) + { + return (writer_guid == ch->writerGUID) && + (last_notified_seq < ch->sequenceNumber); + }); } protected: @@ -167,6 +173,38 @@ class ReaderHistory : public History RTPS_DllAPI void do_release_cache( CacheChange_t* ch) override; + template + inline bool remove_changes_with_pred( + Pred pred) + { + assert(nullptr != mp_reader); + assert(nullptr != mp_mutex); + + // Lock scope + std::vector changes_to_remove; + { + std::lock_guard guard(*mp_mutex); + for (std::vector::iterator chit = m_changes.begin(); chit != m_changes.end(); ++chit) + { + if (pred(*chit)) + { + changes_to_remove.push_back(*chit); + } + } + } + // End lock scope + + bool ret_val = true; + for (CacheChange_t* ch : changes_to_remove) + { + if (!remove_change(ch)) + { + ret_val = false; + } + } + return ret_val; + } + //!Pointer to the reader RTPSReader* mp_reader; diff --git a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp index 11d80e644ef..0f6866c38da 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp @@ -565,13 +565,6 @@ void DataReaderHistory::writer_not_alive( } } -bool DataReaderHistory::writer_unmatched( - const fastrtps::rtps::GUID_t& writer_guid) -{ - static_cast(writer_guid); - return true; -} - } // namespace detail } // namsepace dds } // namespace fastdds diff --git a/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp b/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp index a763108c590..7013847479e 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp @@ -167,9 +167,6 @@ class DataReaderHistory : public eprosima::fastrtps::rtps::ReaderHistory void writer_not_alive( const fastrtps::rtps::GUID_t& writer_guid); - bool writer_unmatched( - const fastrtps::rtps::GUID_t& writer_guid) override; - void check_and_remove_instance( instance_info& instance_info); diff --git a/src/cpp/rtps/history/ReaderHistory.cpp b/src/cpp/rtps/history/ReaderHistory.cpp index 6c4050d8e4a..fd38d66b9c4 100644 --- a/src/cpp/rtps/history/ReaderHistory.cpp +++ b/src/cpp/rtps/history/ReaderHistory.cpp @@ -161,35 +161,22 @@ History::iterator ReaderHistory::remove_change_nts( bool ReaderHistory::remove_changes_with_guid( const GUID_t& a_guid) { - std::vector changes_to_remove; - if (mp_reader == nullptr || mp_mutex == nullptr) { logError(RTPS_READER_HISTORY, "You need to create a Reader with History before removing any changes"); return false; } + if (!remove_changes_with_pred( + [a_guid](CacheChange_t* ch) + { + return a_guid == ch->writerGUID; + })) { - //Lock scope - std::lock_guard guard(*mp_mutex); - for (std::vector::iterator chit = m_changes.begin(); chit != m_changes.end(); ++chit) - { - if ((*chit)->writerGUID == a_guid) - { - changes_to_remove.push_back((*chit)); - } - } - }//End lock scope - - for (std::vector::iterator chit = changes_to_remove.begin(); chit != changes_to_remove.end(); - ++chit) - { - if (!remove_change(*chit)) - { - logError(RTPS_READER_HISTORY, "One of the cachechanged in the GUID removal bulk could not be removed"); - return false; - } + logError(RTPS_READER_HISTORY, "One of the cachechanged in the GUID removal bulk could not be removed"); + return false; } + return true; } diff --git a/src/cpp/rtps/reader/StatefulReader.cpp b/src/cpp/rtps/reader/StatefulReader.cpp index cf43a0e5c89..1f6ab3cc694 100644 --- a/src/cpp/rtps/reader/StatefulReader.cpp +++ b/src/cpp/rtps/reader/StatefulReader.cpp @@ -285,7 +285,7 @@ bool StatefulReader::matched_writer_remove( if (is_alive_) { //Remove cachechanges belonging to the unmatched writer - mp_history->writer_unmatched(writer_guid); + mp_history->writer_unmatched(writer_guid, get_last_notified(writer_guid)); if (liveliness_lease_duration_ < c_TimeInfinite) { diff --git a/src/cpp/rtps/reader/StatelessReader.cpp b/src/cpp/rtps/reader/StatelessReader.cpp index 19e591c1eb1..7f35974be18 100644 --- a/src/cpp/rtps/reader/StatelessReader.cpp +++ b/src/cpp/rtps/reader/StatelessReader.cpp @@ -178,7 +178,7 @@ bool StatelessReader::matched_writer_remove( std::lock_guard guard(mp_mutex); //Remove cachechanges belonging to the unmatched writer - mp_history->writer_unmatched(writer_guid); + mp_history->writer_unmatched(writer_guid, get_last_notified(writer_guid)); if (liveliness_lease_duration_ < c_TimeInfinite) { From 14fcb192da8ebd22a08936ddcde80fea5bc5e208 Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Fri, 29 Oct 2021 12:23:24 +0200 Subject: [PATCH 33/56] Refs 12400. Linters. Signed-off-by: Miguel Company --- .../utils/collections/ResourceLimitedVector.hpp | 4 ++-- .../fastdds/subscriber/history/DataReaderHistory.cpp | 12 ++++++------ src/cpp/rtps/reader/StatefulReader.cpp | 4 ++-- .../fastdds/rtps/history/ReaderHistory.h | 3 ++- 4 files changed, 12 insertions(+), 11 deletions(-) diff --git a/include/fastrtps/utils/collections/ResourceLimitedVector.hpp b/include/fastrtps/utils/collections/ResourceLimitedVector.hpp index f881f13d388..a184129cee8 100644 --- a/include/fastrtps/utils/collections/ResourceLimitedVector.hpp +++ b/include/fastrtps/utils/collections/ResourceLimitedVector.hpp @@ -122,7 +122,7 @@ class ResourceLimitedVector /** * Insert value before pos. * - * @param pos iterator before which the content will be inserted. pos may be the end() iterator. + * @param pos iterator before which the content will be inserted. pos may be the end() iterator. * @param value element value to insert. * * @return Iterator pointing to the inserted value. end() if insertion couldn't be done due to collection limits. @@ -142,7 +142,7 @@ class ResourceLimitedVector /** * Insert value before pos. * - * @param pos iterator before which the content will be inserted. pos may be the end() iterator. + * @param pos iterator before which the content will be inserted. pos may be the end() iterator. * @param value element value to insert. * * @return Iterator pointing to the inserted value. end() if insertion couldn't be done due to collection limits. diff --git a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp index 0f6866c38da..a3d24f1e4f5 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp @@ -126,7 +126,7 @@ DataReaderHistory::DataReaderHistory( { return true; } - + if (type_ != nullptr) { logInfo(SUBSCRIBER, "Getting Key of change with no Key transmitted"); @@ -139,7 +139,7 @@ DataReaderHistory::DataReaderHistory( } logWarning(SUBSCRIBER, "NO KEY in topic: " << topic_name_ - << " and no method to obtain it"; ); + << " and no method to obtain it"; ); return false; }; } @@ -510,9 +510,9 @@ void DataReaderHistory::check_and_remove_instance( DataReaderHistory::instance_info& instance_info) { DataReaderInstance* instance = instance_info.second; - if (instance->cache_changes.empty() && - (InstanceStateKind::ALIVE_INSTANCE_STATE != instance->instance_state) && - instance_info.first.isDefined()) + if (instance->cache_changes.empty() && + (InstanceStateKind::ALIVE_INSTANCE_STATE != instance->instance_state) && + instance_info.first.isDefined()) { keyed_changes_.erase(instance_info.first); instance_info.second = nullptr; @@ -549,7 +549,7 @@ void DataReaderHistory::update_instance_nts( { InstanceCollection::iterator vit; vit = keyed_changes_.find(change->instanceHandle); - + assert(vit != keyed_changes_.end()); vit->second.update_state(change->kind, change->writerGUID); change->reader_info.disposed_generation_count = vit->second.disposed_generation_count; diff --git a/src/cpp/rtps/reader/StatefulReader.cpp b/src/cpp/rtps/reader/StatefulReader.cpp index 1f6ab3cc694..36fd31aa8be 100644 --- a/src/cpp/rtps/reader/StatefulReader.cpp +++ b/src/cpp/rtps/reader/StatefulReader.cpp @@ -797,8 +797,8 @@ bool StatefulReader::change_removed_by_history( } if (a_change->is_fully_assembled() && - !a_change->isRead && - get_last_notified(a_change->writerGUID) >= a_change->sequenceNumber) + !a_change->isRead && + get_last_notified(a_change->writerGUID) >= a_change->sequenceNumber) { if (0 < total_unread_) { diff --git a/test/mock/rtps/ReaderHistory/fastdds/rtps/history/ReaderHistory.h b/test/mock/rtps/ReaderHistory/fastdds/rtps/history/ReaderHistory.h index 09e7ef4061f..089d05650a6 100644 --- a/test/mock/rtps/ReaderHistory/fastdds/rtps/history/ReaderHistory.h +++ b/test/mock/rtps/ReaderHistory/fastdds/rtps/history/ReaderHistory.h @@ -132,7 +132,8 @@ class ReaderHistory } virtual bool writer_unmatched( - const GUID_t& /*writer_guid*/) + const GUID_t& /*writer_guid*/, + const SequenceNumber_t& /*last_notified_seq*/) { return true; } From db2013c224aa3b0be00dbf317229d958bd75967b Mon Sep 17 00:00:00 2001 From: Miguel Barro Date: Tue, 23 Nov 2021 16:57:55 +0100 Subject: [PATCH 34/56] Refs 12758. Fixing compilation warnings on windows. Signed-off-by: Miguel Barro --- include/fastdds/dds/subscriber/SampleInfo.hpp | 2 +- src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp | 2 +- src/cpp/utils/SystemInfo.cpp | 3 ++- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/include/fastdds/dds/subscriber/SampleInfo.hpp b/include/fastdds/dds/subscriber/SampleInfo.hpp index 7af134ba44e..8f9e5ba8da5 100644 --- a/include/fastdds/dds/subscriber/SampleInfo.hpp +++ b/include/fastdds/dds/subscriber/SampleInfo.hpp @@ -89,7 +89,7 @@ struct SampleInfo //! the generation difference between the time the sample was received, and the time the most recent sample was received. //! The most recent sample used for the calculation may or may not be in the returned collection - int32_t absoulte_generation_rank; + int32_t absolute_generation_rank; //! time provided by the DataWriter when the sample was written fastrtps::rtps::Time_t source_timestamp; diff --git a/src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp b/src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp index 66af2a2db7c..5ba529ebed3 100644 --- a/src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp +++ b/src/cpp/fastdds/subscriber/DataReaderImpl/ReadTakeCommand.hpp @@ -220,7 +220,7 @@ struct ReadTakeCommand info.no_writers_generation_count = item->reader_info.no_writers_generation_count; info.sample_rank = 0; info.generation_rank = 0; - info.absoulte_generation_rank = 0; + info.absolute_generation_rank = 0; info.source_timestamp = item->sourceTimestamp; info.reception_timestamp = item->reader_info.receptionTimestamp; info.instance_handle = item->instanceHandle; diff --git a/src/cpp/utils/SystemInfo.cpp b/src/cpp/utils/SystemInfo.cpp index fb98234b3a1..a860540a96d 100644 --- a/src/cpp/utils/SystemInfo.cpp +++ b/src/cpp/utils/SystemInfo.cpp @@ -161,10 +161,11 @@ FileWatchHandle SystemInfo::watch_file( break; } })); -#endif // defined(_WIN32) || defined(__unix__) +#else // defined(_WIN32) || defined(__unix__) static_cast(filename); static_cast(callback); return FileWatchHandle(); +#endif // defined(_WIN32) || defined(__unix__) } void SystemInfo::stop_watching_file( From af7667f0e5481ca03ed460c47eb55b69eb15b1ad Mon Sep 17 00:00:00 2001 From: Miguel Barro Date: Mon, 29 Nov 2021 11:58:24 +0100 Subject: [PATCH 35/56] Refs 12758. Fixing assertion on WriterProxy logic. If we only notify fragmented DATA reception on completion we should only notify removal of fully assembled samples. Signed-off-by: Miguel Barro --- src/cpp/rtps/reader/StatefulReader.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/cpp/rtps/reader/StatefulReader.cpp b/src/cpp/rtps/reader/StatefulReader.cpp index 36fd31aa8be..8f0289b1e02 100644 --- a/src/cpp/rtps/reader/StatefulReader.cpp +++ b/src/cpp/rtps/reader/StatefulReader.cpp @@ -789,15 +789,14 @@ bool StatefulReader::change_removed_by_history( { std::lock_guard guard(mp_mutex); - if (is_alive_) + if (is_alive_ && a_change->is_fully_assembled()) { if (wp != nullptr || matched_writer_lookup(a_change->writerGUID, &wp)) { wp->change_removed_from_history(a_change->sequenceNumber); } - if (a_change->is_fully_assembled() && - !a_change->isRead && + if (!a_change->isRead && get_last_notified(a_change->writerGUID) >= a_change->sequenceNumber) { if (0 < total_unread_) From 9e6e9bbf9219b952186a487f6be191676a574dd1 Mon Sep 17 00:00:00 2001 From: Miguel Barro Date: Tue, 30 Nov 2021 08:01:20 +0100 Subject: [PATCH 36/56] Refs 12758 Fixing DataReaderHistory test that checks DataWriter disposal behaviour. Signed-off-by: Miguel Barro --- test/unittest/dds/subscriber/DataReaderTests.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/unittest/dds/subscriber/DataReaderTests.cpp b/test/unittest/dds/subscriber/DataReaderTests.cpp index 0edc6f06e3e..166f2ac614c 100644 --- a/test/unittest/dds/subscriber/DataReaderTests.cpp +++ b/test/unittest/dds/subscriber/DataReaderTests.cpp @@ -2105,13 +2105,17 @@ TEST_F(DataReaderTests, check_key_history_wholesomeness_on_unmatch) SampleInfoSeq infos; res = data_reader_->take_instance(samples, infos, LENGTH_UNLIMITED, handle_ok_); + + // If the DataWriter is destroyed only the non-notified samples must be removed + // this operation MUST succeed + ASSERT_EQ(res, ReturnCode_t::RETCODE_OK); + + data_reader_->return_loan(samples, infos); }); // Check if the thread hangs // wait for termination std::this_thread::sleep_for(std::chrono::milliseconds(500)); - // check expected result, if query thread hangs res = ReturnCode_t::RETCODE_OK - ASSERT_NE(res, ReturnCode_t::RETCODE_OK); query.join(); } From 30adad94d4d1b9a6e4cdc246dad8c66d15ce62de Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Tue, 7 Dec 2021 08:32:12 +0100 Subject: [PATCH 37/56] Refs 12758. Use move semantics. Signed-off-by: Miguel Company --- include/fastrtps/utils/collections/ResourceLimitedVector.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/fastrtps/utils/collections/ResourceLimitedVector.hpp b/include/fastrtps/utils/collections/ResourceLimitedVector.hpp index a184129cee8..368d891f2b7 100644 --- a/include/fastrtps/utils/collections/ResourceLimitedVector.hpp +++ b/include/fastrtps/utils/collections/ResourceLimitedVector.hpp @@ -156,7 +156,7 @@ class ResourceLimitedVector return end(); } - return collection_.insert(pos, value); + return collection_.insert(pos, std::move(value)); } /** From 2e9595e40bfb54f41f78dbfb4162308cc33d9c93 Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Tue, 7 Dec 2021 09:03:18 +0100 Subject: [PATCH 38/56] Refs 12758. Use ResourceLimitedVector for writers. Signed-off-by: Miguel Company --- .../subscriber/history/DataReaderInstance.hpp | 33 +++++++++++++++---- 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp b/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp index 52b556397d4..9f1f5ebc719 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp @@ -21,7 +21,6 @@ #include #include -#include #include #include @@ -39,14 +38,15 @@ namespace detail { struct DataReaderInstance { using ChangeCollection = eprosima::fastrtps::ResourceLimitedVector; - using WriterCollection = std::map; + using WriterOwnership = std::pair; + using WriterCollection = eprosima::fastrtps::ResourceLimitedVector; //! A vector of DataReader changes belonging to the same instance ChangeCollection cache_changes; //! The list of alive writers for this instance WriterCollection alive_writers; //! GUID and strength of the current maximum strength writer - std::pair current_owner{ {}, 0 }; + WriterOwnership current_owner{ {}, 0 }; //! The time when the group will miss the deadline std::chrono::steady_clock::time_point next_deadline_us; //! Current view state of the instance @@ -126,7 +126,7 @@ struct DataReaderInstance ret_val = true; } - alive_writers[writer_guid] = ownership_strength; + writer_set(writer_guid, ownership_strength); instance_state = InstanceStateKind::ALIVE_INSTANCE_STATE; } @@ -139,7 +139,7 @@ struct DataReaderInstance { bool ret_val = false; - alive_writers[writer_guid] = ownership_strength; + writer_set(writer_guid, ownership_strength); if (ownership_strength >= current_owner.second) { current_owner.first = writer_guid; @@ -160,7 +160,10 @@ struct DataReaderInstance { bool ret_val = false; - alive_writers.erase(writer_guid); + alive_writers.remove_if([&writer_guid](const WriterOwnership& item) + { + return item.first == writer_guid; + }); if (writer_guid == current_owner.first) { @@ -177,6 +180,24 @@ struct DataReaderInstance return ret_val; } + void writer_set( + const fastrtps::rtps::GUID_t& writer_guid, + const uint32_t ownership_strength) + { + auto it = std::find_if(alive_writers.begin(), alive_writers.end(), [&writer_guid](const WriterOwnership& item) + { + return item.first == writer_guid; + }); + if (it == alive_writers.end()) + { + alive_writers.emplace_back(writer_guid, ownership_strength); + } + else + { + it->second = ownership_strength; + } + } + }; } /* namespace detail */ From bd0d14460ddc17b07d8816c26fe33cb0c209be6c Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Tue, 7 Dec 2021 09:44:13 +0100 Subject: [PATCH 39/56] Refs 12758. Apply pre-allocation policies. Signed-off-by: Miguel Company --- .../subscriber/history/DataReaderHistory.cpp | 42 ++++++++++++------- .../subscriber/history/DataReaderHistory.hpp | 5 +++ .../subscriber/history/DataReaderInstance.hpp | 8 ++++ 3 files changed, 41 insertions(+), 14 deletions(-) diff --git a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp index a3d24f1e4f5..377b6ec7024 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp @@ -69,6 +69,7 @@ DataReaderHistory::DataReaderHistory( const TopicDescription& topic, const DataReaderQos& qos) : ReaderHistory(to_history_attributes(type, qos)) + , key_writers_allocation_(qos.reader_resource_limits().matched_publisher_allocation) , history_qos_(qos.history()) , resource_limited_qos_(qos.resource_limits()) , topic_name_(topic.get_name()) @@ -82,24 +83,34 @@ DataReaderHistory::DataReaderHistory( 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) + { + resource_limited_qos_.max_samples_per_instance = std::numeric_limits::max(); + } + if (type_->m_isGetKeyDefined) { get_key_object_ = type_->createData(); + + if (resource_limited_qos_.max_samples_per_instance < std::numeric_limits::max()) + { + key_changes_allocation_.maximum = resource_limited_qos_.max_samples_per_instance; + } } else { resource_limited_qos_.max_instances = 1; resource_limited_qos_.max_samples_per_instance = resource_limited_qos_.max_samples; - } + key_changes_allocation_.initial = resource_limited_qos_.allocated_samples; + key_changes_allocation_.maximum = resource_limited_qos_.max_samples; - 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) - { - resource_limited_qos_.max_samples_per_instance = std::numeric_limits::max(); + keyed_changes_.emplace(c_InstanceHandle_Unknown, + DataReaderInstance{ key_changes_allocation_, key_writers_allocation_ }); } using std::placeholders::_1; @@ -226,7 +237,7 @@ bool DataReaderHistory::received_change_keep_last( bool DataReaderHistory::add_received_change( CacheChange_t* a_change) { - return add_received_change_with_key(a_change, keyed_changes_[c_InstanceHandle_Unknown]); + return add_received_change_with_key(a_change, keyed_changes_.begin()->second); } bool DataReaderHistory::add_received_change_with_key( @@ -312,7 +323,8 @@ bool DataReaderHistory::find_key( if (keyed_changes_.size() < static_cast(resource_limited_qos_.max_instances)) { - vit_out = keyed_changes_.insert(std::make_pair(handle, DataReaderInstance())).first; + vit_out = keyed_changes_.emplace(handle, + DataReaderInstance{key_changes_allocation_, key_writers_allocation_}).first; return true; } @@ -321,7 +333,8 @@ bool DataReaderHistory::find_key( if (vit->second.cache_changes.size() == 0) { keyed_changes_.erase(vit); - vit_out = keyed_changes_.insert(std::make_pair(handle, DataReaderInstance())).first; + vit_out = keyed_changes_.emplace(handle, + DataReaderInstance{ key_changes_allocation_, key_writers_allocation_ }).first; return true; } } @@ -424,12 +437,13 @@ bool DataReaderHistory::set_next_deadline( return false; } std::lock_guard guard(*mp_mutex); - if (keyed_changes_.find(handle) == keyed_changes_.end()) + auto it = keyed_changes_.find(handle); + if (it == keyed_changes_.end()) { return false; } - keyed_changes_[handle].next_deadline_us = next_deadline_us; + it->second.next_deadline_us = next_deadline_us; return true; } diff --git a/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp b/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp index 7013847479e..fab0bb70a4d 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp @@ -40,6 +40,7 @@ #include #include +#include #include "DataReaderInstance.hpp" @@ -174,6 +175,10 @@ class DataReaderHistory : public eprosima::fastrtps::rtps::ReaderHistory using InstanceCollection = std::map; + //!Resource limits for allocating the array of changes per instance + eprosima::fastrtps::ResourceLimitedContainerConfig key_changes_allocation_; + //!Resource limits for allocating the array of alive writers per instance + eprosima::fastrtps::ResourceLimitedContainerConfig key_writers_allocation_; //!Map where keys are instance handles and values vectors of cache changes InstanceCollection keyed_changes_; //!HistoryQosPolicy values. diff --git a/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp b/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp index 9f1f5ebc719..2553e8dad08 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp @@ -58,6 +58,14 @@ struct DataReaderInstance //! Current no_writers generation of the instance int32_t no_writers_generation_count = 0; + DataReaderInstance( + const eprosima::fastrtps::ResourceLimitedContainerConfig& changes_allocation, + const eprosima::fastrtps::ResourceLimitedContainerConfig& writers_allocation) + : cache_changes(changes_allocation) + , alive_writers(writers_allocation) + { + } + bool update_state( const fastrtps::rtps::ChangeKind_t change_kind, const fastrtps::rtps::GUID_t& writer_guid, From f5f8fb5dce2d79ce2517bd34260cda18dcdb1dfe Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Tue, 7 Dec 2021 10:17:20 +0100 Subject: [PATCH 40/56] Refs 12758. Uncrustify. Signed-off-by: Miguel Company --- src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp | 2 +- src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp index 377b6ec7024..476f95a6461 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp @@ -110,7 +110,7 @@ DataReaderHistory::DataReaderHistory( key_changes_allocation_.maximum = resource_limited_qos_.max_samples; keyed_changes_.emplace(c_InstanceHandle_Unknown, - DataReaderInstance{ key_changes_allocation_, key_writers_allocation_ }); + DataReaderInstance{ key_changes_allocation_, key_writers_allocation_ }); } using std::placeholders::_1; diff --git a/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp b/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp index 2553e8dad08..ed5a4e267ac 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderInstance.hpp @@ -193,9 +193,9 @@ struct DataReaderInstance const uint32_t ownership_strength) { auto it = std::find_if(alive_writers.begin(), alive_writers.end(), [&writer_guid](const WriterOwnership& item) - { - return item.first == writer_guid; - }); + { + return item.first == writer_guid; + }); if (it == alive_writers.end()) { alive_writers.emplace_back(writer_guid, ownership_strength); From 737c241157bd1bc39e666b51711418374d35c976 Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Tue, 7 Dec 2021 10:49:52 +0100 Subject: [PATCH 41/56] Refs 12758. PubSubReader. Account for writer_guid on last_seq checks. Signed-off-by: Miguel Company --- test/blackbox/api/dds-pim/PubSubReader.hpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/test/blackbox/api/dds-pim/PubSubReader.hpp b/test/blackbox/api/dds-pim/PubSubReader.hpp index 8ea3a87f6a6..9aaa831be9e 100644 --- a/test/blackbox/api/dds-pim/PubSubReader.hpp +++ b/test/blackbox/api/dds-pim/PubSubReader.hpp @@ -1566,8 +1566,9 @@ class PubSubReader std::unique_lock lock(mutex_); // Check order of changes. - ASSERT_LT(last_seq[info.instance_handle], info.sample_identity.sequence_number()); - last_seq[info.instance_handle] = info.sample_identity.sequence_number(); + LastSeqInfo seq_info{ info.instance_handle, info.sample_identity.writer_guid() }; + ASSERT_LT(last_seq[seq_info], info.sample_identity.sequence_number()); + last_seq[seq_info] = info.sample_identity.sequence_number(); if (info.instance_state == eprosima::fastdds::dds::ALIVE_INSTANCE_STATE) { @@ -1652,7 +1653,8 @@ class PubSubReader unsigned int participant_matched_; std::atomic receiving_; eprosima::fastdds::dds::TypeSupport type_; - std::map last_seq; + using LastSeqInfo = std::pair; + std::map last_seq; size_t current_processed_count_; size_t number_samples_expected_; bool discovery_result_; From c4d11dd4a6d2830cd0969d17b3a8d515f3179d30 Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Mon, 13 Dec 2021 09:35:54 +0100 Subject: [PATCH 42/56] Refs 12758. Added can_change_be_added_nts. Signed-off-by: Miguel Company --- .../subscriber/history/DataReaderHistory.cpp | 17 +++++++++++++++ .../subscriber/history/DataReaderHistory.hpp | 21 +++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp index 476f95a6461..a32ee87a962 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp @@ -164,6 +164,23 @@ DataReaderHistory::~DataReaderHistory() } } +bool DataReaderHistory::can_change_be_added_nts( + const GUID_t& writer_guid, + uint32_t total_payload_size, + size_t unknown_missing_changes_up_to, + bool& will_never_be_accepted) const +{ + if (!ReaderHistory::can_change_be_added_nts(writer_guid, total_payload_size, unknown_missing_changes_up_to, + will_never_be_accepted)) + { + return false; + } + + will_never_be_accepted = false; + return (KEEP_ALL_HISTORY_QOS != history_qos_.kind) || + (m_changes.size() + unknown_missing_changes_up_to < static_cast(resource_limited_qos_.max_samples)); +} + bool DataReaderHistory::received_change( CacheChange_t* a_change, size_t unknown_missing_changes_up_to) diff --git a/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp b/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp index fab0bb70a4d..2f4d2e731bf 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp @@ -59,6 +59,7 @@ class DataReaderHistory : public eprosima::fastrtps::rtps::ReaderHistory using MemoryManagementPolicy_t = eprosima::fastrtps::rtps::MemoryManagementPolicy_t; using InstanceHandle_t = eprosima::fastrtps::rtps::InstanceHandle_t; using CacheChange_t = eprosima::fastrtps::rtps::CacheChange_t; + using GUID_t = eprosima::fastrtps::rtps::GUID_t; using instance_info = std::pair; @@ -86,6 +87,26 @@ class DataReaderHistory : public eprosima::fastrtps::rtps::ReaderHistory const_iterator removal, bool release = true) override; + /** + * Check if a new change can be added to this history. + * + * @param [in] writer_guid GUID of the writer where the change came from. + * @param [in] total_payload_size Total payload size of the incoming change. + * @param [in] unknown_missing_changes_up_to The number of changes from the same writer with a lower sequence + * number that could potentially be received in the future. + * @param [out] will_never_be_accepted When the method returns @c false, this parameter will inform + * whether the change could be accepted in the future or not. + * + * @pre change should not be present in the history + * + * @return Whether a call to received_change will succeed when called with the same arguments. + */ + bool can_change_be_added_nts( + const GUID_t& writer_guid, + uint32_t total_payload_size, + size_t unknown_missing_changes_up_to, + bool& will_never_be_accepted) const override; + /** * Called when a change is received by the Subscriber. Will add the change to the history. * @pre Change should not be already present in the history. From fb10bcbe84f72322ac31ddf8aed6a7110c734f4a Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Mon, 13 Dec 2021 10:01:34 +0100 Subject: [PATCH 43/56] Refs 12758. Removed unused method. Signed-off-by: Miguel Company --- src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp | 6 ------ src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp | 3 --- 2 files changed, 9 deletions(-) diff --git a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp index a32ee87a962..0ca4a64db51 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp @@ -251,12 +251,6 @@ bool DataReaderHistory::received_change_keep_last( return false; } -bool DataReaderHistory::add_received_change( - CacheChange_t* a_change) -{ - return add_received_change_with_key(a_change, keyed_changes_.begin()->second); -} - bool DataReaderHistory::add_received_change_with_key( CacheChange_t* a_change, DataReaderInstance& instance) diff --git a/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp b/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp index 2f4d2e731bf..fedb86679f5 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp @@ -260,9 +260,6 @@ class DataReaderHistory : public eprosima::fastrtps::rtps::ReaderHistory size_t unknown_missing_changes_up_to); ///@} - bool add_received_change( - CacheChange_t* a_change); - bool add_received_change_with_key( CacheChange_t* a_change, DataReaderInstance& instance); From 0bd9fcbe8b2bb828ff09603aeb4b03b8176343df Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Mon, 13 Dec 2021 11:31:10 +0100 Subject: [PATCH 44/56] Refs 12758. Always use completed changes for key computation. Signed-off-by: Miguel Company --- .../subscriber/history/DataReaderHistory.cpp | 154 +++++++++++++++--- .../subscriber/history/DataReaderHistory.hpp | 37 +++-- 2 files changed, 154 insertions(+), 37 deletions(-) diff --git a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp index 0ca4a64db51..4760f491e15 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp @@ -120,6 +120,10 @@ DataReaderHistory::DataReaderHistory( std::bind(&DataReaderHistory::received_change_keep_all, this, _1, _2) : std::bind(&DataReaderHistory::received_change_keep_last, this, _1, _2); + complete_fn_ = qos.history().kind == KEEP_ALL_HISTORY_QOS ? + std::bind(&DataReaderHistory::completed_change_keep_all, this, _1, _2) : + std::bind(&DataReaderHistory::completed_change_keep_last, this, _1, _2); + if (!has_keys_) { compute_key_for_change_fn_ = [](CacheChange_t* change) @@ -138,6 +142,11 @@ DataReaderHistory::DataReaderHistory( return true; } + if (!a_change->is_fully_assembled()) + { + return false; + } + if (type_ != nullptr) { logInfo(SUBSCRIBER, "Getting Key of change with no Key transmitted"); @@ -199,8 +208,14 @@ bool DataReaderHistory::received_change_keep_all( CacheChange_t* a_change, size_t unknown_missing_changes_up_to) { + if (!compute_key_for_change_fn_(a_change)) + { + // Store the sample temporally only in ReaderHistory. When completed it will be stored in SubscriberHistory too. + return add_to_reader_history_if_not_full(a_change); + } + InstanceCollection::iterator vit; - if (find_key_for_change(a_change, vit)) + if (find_key(a_change->instanceHandle, vit)) { DataReaderInstance::ChangeCollection& instance_changes = vit->second.cache_changes; size_t total_size = instance_changes.size() + unknown_missing_changes_up_to; @@ -219,8 +234,14 @@ bool DataReaderHistory::received_change_keep_last( CacheChange_t* a_change, size_t /* unknown_missing_changes_up_to */) { + if (!compute_key_for_change_fn_(a_change)) + { + // Store the sample temporally only in ReaderHistory. When completed it will be stored in SubscriberHistory too. + return add_to_reader_history_if_not_full(a_change); + } + InstanceCollection::iterator vit; - if (find_key_for_change(a_change, vit)) + if (find_key(a_change->instanceHandle, vit)) { bool add = false; DataReaderInstance::ChangeCollection& instance_changes = vit->second.cache_changes; @@ -254,44 +275,49 @@ bool DataReaderHistory::received_change_keep_last( bool DataReaderHistory::add_received_change_with_key( CacheChange_t* a_change, DataReaderInstance& instance) +{ + if (add_to_reader_history_if_not_full(a_change)) + { + add_to_instance(a_change, instance); + return true; + } + + return false; +} + +bool DataReaderHistory::add_to_reader_history_if_not_full( + CacheChange_t* a_change) { if (m_isHistoryFull) { - // Discarting the sample. + // Discarding the sample. logWarning(SUBSCRIBER, "Attempting to add Data to Full ReaderHistory: " << type_name_); return false; } - if (add_change(a_change)) + bool ret_value = add_change(a_change); + if (m_changes.size() == static_cast(m_att.maximumReservedCaches)) { - if (m_changes.size() == static_cast(m_att.maximumReservedCaches)) - { - m_isHistoryFull = true; - } - - //ADD TO KEY VECTOR - DataReaderCacheChange item = a_change; - eprosima::utilities::collections::sorted_vector_insert(instance.cache_changes, item, - [](const DataReaderCacheChange& lhs, const DataReaderCacheChange& rhs) - { - return lhs->sourceTimestamp < rhs->sourceTimestamp; - }); - - logInfo(SUBSCRIBER, mp_reader->getGuid().entityId - << ": Change " << a_change->sequenceNumber << " added from: " - << a_change->writerGUID << " with KEY: " << a_change->instanceHandle; ); - - return true; + m_isHistoryFull = true; } - - return false; + return ret_value; } -bool DataReaderHistory::find_key_for_change( +void DataReaderHistory::add_to_instance( CacheChange_t* a_change, - InstanceCollection::iterator& map_it) + DataReaderInstance& instance) { - return compute_key_for_change_fn_(a_change) && find_key(a_change->instanceHandle, map_it); + // ADD TO KEY VECTOR + DataReaderCacheChange item = a_change; + eprosima::utilities::collections::sorted_vector_insert(instance.cache_changes, item, + [](const DataReaderCacheChange& lhs, const DataReaderCacheChange& rhs) + { + return lhs->sourceTimestamp < rhs->sourceTimestamp; + }); + + logInfo(SUBSCRIBER, mp_reader->getGuid().entityId + << ": Change " << a_change->sequenceNumber << " added from: " + << a_change->writerGUID << " with KEY: " << a_change->instanceHandle; ); } bool DataReaderHistory::get_first_untaken_info( @@ -569,6 +595,80 @@ ReaderHistory::iterator DataReaderHistory::remove_change_nts( return ReaderHistory::remove_change_nts(removal, release); } +bool DataReaderHistory::completed_change( + CacheChange_t* change) +{ + bool ret_value = true; + + if (!change->instanceHandle.isDefined()) + { + InstanceCollection::iterator vit; + ret_value = compute_key_for_change_fn_(change) && find_key(change->instanceHandle, vit); + if (ret_value) + { + ret_value = complete_fn_(change, vit->second); + } + + if (!ret_value) + { + const_iterator chit = find_change_nts(change); + if (chit != changesEnd()) + { + m_isHistoryFull = false; + remove_change_nts(chit); + } + else + { + logError(SUBSCRIBER, "Change should exist but didn't find it"); + } + } + } + + return ret_value; +} + +bool DataReaderHistory::completed_change_keep_all( + CacheChange_t* change, + DataReaderInstance& instance) +{ + DataReaderInstance::ChangeCollection& instance_changes = instance.cache_changes; + if (instance_changes.size() < static_cast(resource_limited_qos_.max_samples_per_instance)) + { + add_to_instance(change, instance); + return true; + } + + logWarning(SUBSCRIBER, "Change not added due to maximum number of samples per instance"); + return false; +} + +bool DataReaderHistory::completed_change_keep_last( + CacheChange_t* change, + DataReaderInstance& instance) +{ + bool add = false; + DataReaderInstance::ChangeCollection& instance_changes = instance.cache_changes; + if (instance_changes.size() < static_cast(history_qos_.depth)) + { + add = true; + } + else + { + // Try to substitute the oldest sample. + + // As the instance should be ordered following the presentation QoS, we can always remove the first one. + add = remove_change_sub(instance_changes.at(0)); + } + + if (add) + { + add_to_instance(change, instance); + return true; + } + + return false; +} + void DataReaderHistory::update_instance_nts( CacheChange_t* const change) { diff --git a/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp b/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp index fedb86679f5..bad9cbd48c7 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp @@ -118,6 +118,15 @@ class DataReaderHistory : public eprosima::fastrtps::rtps::ReaderHistory CacheChange_t* change, size_t unknown_missing_changes_up_to) override; + /** + * Called when a fragmented change is received completely by the Subscriber. Will find its instance and store it. + * @pre Change should be already present in the history. + * @param[in] change The received change + * @return + */ + bool completed_change( + CacheChange_t* change) override; + /** * @brief Returns information about the first untaken sample. * @param [out] info SampleInfo structure to store first untaken sample information. @@ -222,6 +231,8 @@ class DataReaderHistory : public eprosima::fastrtps::rtps::ReaderHistory std::function receive_fn_; /// Function to compute the instance handle of a received change std::function compute_key_for_change_fn_; + /// Function processing a completed fragmented change + std::function complete_fn_; /** * @brief Method that finds a key in m_keyedChanges or tries to add it if not found @@ -233,16 +244,6 @@ class DataReaderHistory : public eprosima::fastrtps::rtps::ReaderHistory const InstanceHandle_t& handle, InstanceCollection::iterator& map_it); - /** - * @brief Method that finds a key in m_keyedChanges or tries to add it if not found - * @param a_change The change to get the key from - * @param map_it A map iterator to the given key - * @return True if it was found or could be added to the map - */ - bool find_key_for_change( - CacheChange_t* a_change, - InstanceCollection::iterator& map_it); - /** * @name Variants of incoming change processing. * Will be called with the history mutex taken. @@ -258,11 +259,27 @@ class DataReaderHistory : public eprosima::fastrtps::rtps::ReaderHistory bool received_change_keep_last( CacheChange_t* change, size_t unknown_missing_changes_up_to); + + bool completed_change_keep_all( + CacheChange_t* change, + DataReaderInstance& instance); + + bool completed_change_keep_last( + CacheChange_t* change, + DataReaderInstance& instance); ///@} bool add_received_change_with_key( CacheChange_t* a_change, DataReaderInstance& instance); + + bool add_to_reader_history_if_not_full( + CacheChange_t* a_change); + + void add_to_instance( + CacheChange_t* a_change, + DataReaderInstance& instance); + }; } // namespace detail From 6ecf19385c72f22c44909ab98e7fd9a10acc303c Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Mon, 13 Dec 2021 16:29:26 +0100 Subject: [PATCH 45/56] Refs 12758. Fixed ResourceLimitedVector::insert. Signed-off-by: Miguel Company --- .../fastrtps/utils/collections/ResourceLimitedVector.hpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/include/fastrtps/utils/collections/ResourceLimitedVector.hpp b/include/fastrtps/utils/collections/ResourceLimitedVector.hpp index 368d891f2b7..56ee45e673b 100644 --- a/include/fastrtps/utils/collections/ResourceLimitedVector.hpp +++ b/include/fastrtps/utils/collections/ResourceLimitedVector.hpp @@ -131,12 +131,13 @@ class ResourceLimitedVector const_iterator pos, const value_type& value) { - if (capacity() >= configuration_.maximum) + auto dist = std::distance(collection_.cbegin(), pos); + if (!ensure_capacity()) { return end(); } - return collection_.insert(pos, value); + return collection_.insert(collection_.cbegin() + dist, value); } /** @@ -151,7 +152,7 @@ class ResourceLimitedVector const_iterator pos, value_type&& value) { - if (capacity() >= configuration_.maximum) + if (!ensure_capacity()) { return end(); } From 134f9b17d2d0ef692015c21a516644e800d81e37 Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Mon, 13 Dec 2021 16:30:43 +0100 Subject: [PATCH 46/56] Refs 12758. Uncrustify. Signed-off-by: Miguel Company --- src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp index 4760f491e15..bc3f5f5edb6 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp @@ -121,8 +121,8 @@ DataReaderHistory::DataReaderHistory( std::bind(&DataReaderHistory::received_change_keep_last, this, _1, _2); complete_fn_ = qos.history().kind == KEEP_ALL_HISTORY_QOS ? - std::bind(&DataReaderHistory::completed_change_keep_all, this, _1, _2) : - std::bind(&DataReaderHistory::completed_change_keep_last, this, _1, _2); + std::bind(&DataReaderHistory::completed_change_keep_all, this, _1, _2) : + std::bind(&DataReaderHistory::completed_change_keep_last, this, _1, _2); if (!has_keys_) { From 1b42b23c48d9606377f9781a0024dfa0ca55ff25 Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Mon, 13 Dec 2021 16:59:30 +0100 Subject: [PATCH 47/56] Refs 12758. Avoid dynamic allocation inside remove_changes_with_pred. Signed-off-by: Miguel Company --- include/fastdds/rtps/history/ReaderHistory.h | 25 ++++---------------- 1 file changed, 5 insertions(+), 20 deletions(-) diff --git a/include/fastdds/rtps/history/ReaderHistory.h b/include/fastdds/rtps/history/ReaderHistory.h index 3341739c991..b57d4fb2584 100644 --- a/include/fastdds/rtps/history/ReaderHistory.h +++ b/include/fastdds/rtps/history/ReaderHistory.h @@ -180,29 +180,14 @@ class ReaderHistory : public History assert(nullptr != mp_reader); assert(nullptr != mp_mutex); - // Lock scope - std::vector changes_to_remove; + std::lock_guard guard(*mp_mutex); + std::vector::iterator new_end = std::remove_if(m_changes.begin(), m_changes.end(), pred); + while (new_end != m_changes.end()) { - std::lock_guard guard(*mp_mutex); - for (std::vector::iterator chit = m_changes.begin(); chit != m_changes.end(); ++chit) - { - if (pred(*chit)) - { - changes_to_remove.push_back(*chit); - } - } + new_end = remove_change_nts(new_end); } - // End lock scope - bool ret_val = true; - for (CacheChange_t* ch : changes_to_remove) - { - if (!remove_change(ch)) - { - ret_val = false; - } - } - return ret_val; + return true; } //!Pointer to the reader From 574622bf97dfb588eb00376fb6edcdeef859e22d Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Mon, 13 Dec 2021 17:02:20 +0100 Subject: [PATCH 48/56] Refs 12758. Optimization on DataReaderHistory::remove_change_nts. Signed-off-by: Miguel Company --- src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp index bc3f5f5edb6..1abc0063807 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp @@ -585,10 +585,7 @@ ReaderHistory::iterator DataReaderHistory::remove_change_nts( assert(it != keyed_changes_.end()); auto& c = it->second.cache_changes; - c.erase(std::remove_if(c.begin(), c.end(), [p_sample](DataReaderCacheChange& elem) - { - return elem == p_sample; - }), c.end()); + c.erase(std::remove(c.begin(), c.end(), p_sample), c.end()); } // call the base class From a7f2ae14d29c1cae37d1ef272baadcede1a6a95b Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Mon, 13 Dec 2021 17:17:07 +0100 Subject: [PATCH 49/56] Refs 12758. Method writer_unmatched documented and improved. Signed-off-by: Miguel Company --- include/fastdds/rtps/history/ReaderHistory.h | 17 ++++++++++++----- src/cpp/rtps/history/ReaderHistory.cpp | 14 +++++--------- .../fastdds/rtps/history/ReaderHistory.h | 3 +-- 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/include/fastdds/rtps/history/ReaderHistory.h b/include/fastdds/rtps/history/ReaderHistory.h index b57d4fb2584..6ac0723d619 100644 --- a/include/fastdds/rtps/history/ReaderHistory.h +++ b/include/fastdds/rtps/history/ReaderHistory.h @@ -152,11 +152,20 @@ class ReaderHistory : public History CacheChange_t** min_change, const GUID_t& writerGuid); - RTPS_DllAPI virtual bool writer_unmatched( + /** + * Called when a writer is unmatched from the reader holding this history. + * + * This method will remove all the changes on the history that came from the writer being unmatched and which have + * not yet been notified to the user. + * + * @param writer_guid GUID of the writer being unmatched. + * @param last_notified_seq Last sequence number from the specified writer that was notified to the user. + */ + RTPS_DllAPI virtual void writer_unmatched( const GUID_t& writer_guid, const SequenceNumber_t& last_notified_seq) { - return remove_changes_with_pred( + remove_changes_with_pred( [writer_guid, last_notified_seq](CacheChange_t* ch) { return (writer_guid == ch->writerGUID) && @@ -174,7 +183,7 @@ class ReaderHistory : public History CacheChange_t* ch) override; template - inline bool remove_changes_with_pred( + inline void remove_changes_with_pred( Pred pred) { assert(nullptr != mp_reader); @@ -186,8 +195,6 @@ class ReaderHistory : public History { new_end = remove_change_nts(new_end); } - - return true; } //!Pointer to the reader diff --git a/src/cpp/rtps/history/ReaderHistory.cpp b/src/cpp/rtps/history/ReaderHistory.cpp index fd38d66b9c4..ec8bd9d15ce 100644 --- a/src/cpp/rtps/history/ReaderHistory.cpp +++ b/src/cpp/rtps/history/ReaderHistory.cpp @@ -167,15 +167,11 @@ bool ReaderHistory::remove_changes_with_guid( return false; } - if (!remove_changes_with_pred( - [a_guid](CacheChange_t* ch) - { - return a_guid == ch->writerGUID; - })) - { - logError(RTPS_READER_HISTORY, "One of the cachechanged in the GUID removal bulk could not be removed"); - return false; - } + remove_changes_with_pred( + [a_guid](CacheChange_t* ch) + { + return a_guid == ch->writerGUID; + }); return true; } diff --git a/test/mock/rtps/ReaderHistory/fastdds/rtps/history/ReaderHistory.h b/test/mock/rtps/ReaderHistory/fastdds/rtps/history/ReaderHistory.h index 089d05650a6..26ec4dbdf01 100644 --- a/test/mock/rtps/ReaderHistory/fastdds/rtps/history/ReaderHistory.h +++ b/test/mock/rtps/ReaderHistory/fastdds/rtps/history/ReaderHistory.h @@ -131,11 +131,10 @@ class ReaderHistory return m_changes.erase(removal); } - virtual bool writer_unmatched( + virtual void writer_unmatched( const GUID_t& /*writer_guid*/, const SequenceNumber_t& /*last_notified_seq*/) { - return true; } HistoryAttributes m_att; From 6149ee3bc43ca285709c0d98bfa794bb637feaee Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Tue, 14 Dec 2021 18:51:20 +0100 Subject: [PATCH 50/56] Refs 12758. Do not complete changes for non-keyed topics. Signed-off-by: Miguel Company --- src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp index 1abc0063807..ff2658d24cb 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp @@ -603,7 +603,7 @@ bool DataReaderHistory::completed_change( ret_value = compute_key_for_change_fn_(change) && find_key(change->instanceHandle, vit); if (ret_value) { - ret_value = complete_fn_(change, vit->second); + ret_value = !change->instanceHandle.isDefined() || complete_fn_(change, vit->second); } if (!ret_value) From ec5336c8eb350a97fc052848316d7707b5a7e654 Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Wed, 15 Dec 2021 10:35:52 +0100 Subject: [PATCH 51/56] Refs 12758. Do not remove incomplete changes for keyed topics. Signed-off-by: Miguel Company --- .../subscriber/history/DataReaderHistory.cpp | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp index ff2658d24cb..3109db28cdb 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp @@ -578,14 +578,17 @@ ReaderHistory::iterator DataReaderHistory::remove_change_nts( { CacheChange_t* p_sample = *removal; - // clean any references to this CacheChange in the key state collection - auto it = keyed_changes_.find(p_sample->instanceHandle); + if (!has_keys_ || p_sample->is_fully_assembled()) + { + // clean any references to this CacheChange in the key state collection + auto it = keyed_changes_.find(p_sample->instanceHandle); - // if keyed and in history must be in the map - assert(it != keyed_changes_.end()); + // if keyed and in history must be in the map + assert(it != keyed_changes_.end()); - auto& c = it->second.cache_changes; - c.erase(std::remove(c.begin(), c.end(), p_sample), c.end()); + auto& c = it->second.cache_changes; + c.erase(std::remove(c.begin(), c.end(), p_sample), c.end()); + } } // call the base class From 64e52e50211dfa1475b24895273c2c44fa73760b Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Thu, 16 Dec 2021 07:40:12 +0100 Subject: [PATCH 52/56] Refs 12758. Different removal policy on ReaderHistory and DataReaderHistory. Signed-off-by: Miguel Company --- include/fastdds/rtps/history/ReaderHistory.h | 10 +--------- .../subscriber/history/DataReaderHistory.cpp | 11 +++++++++++ .../subscriber/history/DataReaderHistory.hpp | 15 +++++++++++++++ src/cpp/rtps/history/ReaderHistory.cpp | 12 ++++++++++++ .../fastdds/rtps/history/ReaderHistory.h | 6 ++++++ 5 files changed, 45 insertions(+), 9 deletions(-) diff --git a/include/fastdds/rtps/history/ReaderHistory.h b/include/fastdds/rtps/history/ReaderHistory.h index 6ac0723d619..eb2a9fa6c39 100644 --- a/include/fastdds/rtps/history/ReaderHistory.h +++ b/include/fastdds/rtps/history/ReaderHistory.h @@ -163,15 +163,7 @@ class ReaderHistory : public History */ RTPS_DllAPI virtual void writer_unmatched( const GUID_t& writer_guid, - const SequenceNumber_t& last_notified_seq) - { - remove_changes_with_pred( - [writer_guid, last_notified_seq](CacheChange_t* ch) - { - return (writer_guid == ch->writerGUID) && - (last_notified_seq < ch->sequenceNumber); - }); - } + const SequenceNumber_t& last_notified_seq); protected: diff --git a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp index 3109db28cdb..9ca5826bf1c 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp @@ -380,6 +380,17 @@ bool DataReaderHistory::find_key( return false; } +void DataReaderHistory::writer_unmatched( + const GUID_t& writer_guid, + const SequenceNumber_t& last_notified_seq) +{ + remove_changes_with_pred( + [&writer_guid, &last_notified_seq](CacheChange_t* ch) + { + return (writer_guid == ch->writerGUID) && (last_notified_seq < ch->sequenceNumber); + }); +} + bool DataReaderHistory::remove_change_sub( CacheChange_t* change) { diff --git a/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp b/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp index bad9cbd48c7..074057095cf 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp @@ -36,6 +36,7 @@ #include #include #include +#include #include #include @@ -60,6 +61,7 @@ class DataReaderHistory : public eprosima::fastrtps::rtps::ReaderHistory using InstanceHandle_t = eprosima::fastrtps::rtps::InstanceHandle_t; using CacheChange_t = eprosima::fastrtps::rtps::CacheChange_t; using GUID_t = eprosima::fastrtps::rtps::GUID_t; + using SequenceNumber_t = eprosima::fastrtps::rtps::SequenceNumber_t; using instance_info = std::pair; @@ -153,6 +155,19 @@ class DataReaderHistory : public eprosima::fastrtps::rtps::ReaderHistory CacheChange_t* change, DataReaderInstance::ChangeCollection::iterator& it); + /** + * Called when a writer is unmatched from the reader holding this history. + * + * This method will remove all the changes on the history that came from the writer being unmatched and which have + * not yet been notified to the user. + * + * @param writer_guid GUID of the writer being unmatched. + * @param last_notified_seq Last sequence number from the specified writer that was notified to the user. + */ + void writer_unmatched( + const GUID_t& writer_guid, + const SequenceNumber_t& last_notified_seq) override; + /** * @brief A method to set the next deadline for the given instance * @param handle The handle to the instance diff --git a/src/cpp/rtps/history/ReaderHistory.cpp b/src/cpp/rtps/history/ReaderHistory.cpp index ec8bd9d15ce..a7baba15b44 100644 --- a/src/cpp/rtps/history/ReaderHistory.cpp +++ b/src/cpp/rtps/history/ReaderHistory.cpp @@ -158,6 +158,18 @@ History::iterator ReaderHistory::remove_change_nts( return ret_val; } +void ReaderHistory::writer_unmatched( + const GUID_t& writer_guid, + const SequenceNumber_t& last_notified_seq) +{ + static_cast(last_notified_seq); + remove_changes_with_pred( + [&writer_guid](CacheChange_t* ch) + { + return writer_guid == ch->writerGUID; + }); +} + bool ReaderHistory::remove_changes_with_guid( const GUID_t& a_guid) { diff --git a/test/mock/rtps/ReaderHistory/fastdds/rtps/history/ReaderHistory.h b/test/mock/rtps/ReaderHistory/fastdds/rtps/history/ReaderHistory.h index 26ec4dbdf01..40eff0b3a34 100644 --- a/test/mock/rtps/ReaderHistory/fastdds/rtps/history/ReaderHistory.h +++ b/test/mock/rtps/ReaderHistory/fastdds/rtps/history/ReaderHistory.h @@ -141,6 +141,12 @@ class ReaderHistory protected: + template + inline void remove_changes_with_pred( + Pred pred) + { + } + RTPSReader* mp_reader; RecursiveTimedMutex* mp_mutex; std::vector m_changes; From f956e800cc1ef0390e41e14e76898b1495dee18e Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Thu, 16 Dec 2021 09:12:12 +0100 Subject: [PATCH 53/56] Refs 12758. Fix unused parameter warning. Signed-off-by: Miguel Company --- .../rtps/ReaderHistory/fastdds/rtps/history/ReaderHistory.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/mock/rtps/ReaderHistory/fastdds/rtps/history/ReaderHistory.h b/test/mock/rtps/ReaderHistory/fastdds/rtps/history/ReaderHistory.h index 40eff0b3a34..76e9f76aaf6 100644 --- a/test/mock/rtps/ReaderHistory/fastdds/rtps/history/ReaderHistory.h +++ b/test/mock/rtps/ReaderHistory/fastdds/rtps/history/ReaderHistory.h @@ -143,7 +143,7 @@ class ReaderHistory template inline void remove_changes_with_pred( - Pred pred) + Pred) { } From 134afc6f22faeabed6f453c6cf858c4d3ccc878a Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Mon, 20 Dec 2021 07:50:50 +0100 Subject: [PATCH 54/56] Refs 12758. Removed unused header. Signed-off-by: Miguel Company --- src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp b/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp index 074057095cf..97349e537be 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp @@ -23,7 +23,6 @@ #include #include #include -#include #include #include From ed042353cc8b028750232e568b6097b7dc2fac45 Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Mon, 20 Dec 2021 07:52:36 +0100 Subject: [PATCH 55/56] Refs 12758. Doxydoc improvements. Signed-off-by: Miguel Company --- .../subscriber/history/DataReaderHistory.hpp | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp b/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp index 97349e537be..b34bb53f403 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp @@ -196,8 +196,7 @@ class DataReaderHistory : public eprosima::fastrtps::rtps::ReaderHistory * - @c first is a boolean indicating if an instance was found * - @c second is a pair where: * - @c first is the handle of the returned instance - * - @c second is a pointer to a std::vector with the list of changes for the - * returned instance + * - @c second is a pointer to a DataReaderInstance that holds information about the returned instance * * @remarks When used on a NO_KEY topic, an instance will only be returned when called with * `handle = HANDLE_NIL` and `exact = false`. @@ -273,7 +272,17 @@ class DataReaderHistory : public eprosima::fastrtps::rtps::ReaderHistory bool received_change_keep_last( CacheChange_t* change, size_t unknown_missing_changes_up_to); + ///@} + /** + * @name Variants of change reconstruction completion processing. + * Will be called with the history mutex taken. + * @param change The change for which the last missing fragment has been processed. + * @param instance Instance where the change should be added. + * @return true when the change was added to the instance. + * @return false when the change could not be added to the instance and has been removed from the history. + */ + ///@{ bool completed_change_keep_all( CacheChange_t* change, DataReaderInstance& instance); From 11fee0aea5801c4cc449145a068beb15f7404cc8 Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Mon, 20 Dec 2021 08:53:34 +0100 Subject: [PATCH 56/56] Refs 12758. Linters. Signed-off-by: Miguel Company --- src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp b/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp index b34bb53f403..629ce5f8f1f 100644 --- a/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp +++ b/src/cpp/fastdds/subscriber/history/DataReaderHistory.hpp @@ -282,7 +282,7 @@ class DataReaderHistory : public eprosima::fastrtps::rtps::ReaderHistory * @return true when the change was added to the instance. * @return false when the change could not be added to the instance and has been removed from the history. */ - ///@{ + ///@{ bool completed_change_keep_all( CacheChange_t* change, DataReaderInstance& instance);