Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Discovery Server fix reconnection [12522] #2246

Merged
merged 7 commits into from
Nov 22, 2021
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 18 additions & 23 deletions src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -870,7 +870,7 @@ void DiscoveryDataBase::create_writers_from_change_(
}
else
{
logError(DISCOVERY_DATABASE, "Writer " << writer_guid << " as no associated participant. Skipping");
logError(DISCOVERY_DATABASE, "Writer " << writer_guid << " has no associated participant. Skipping");
return;
}

Expand Down Expand Up @@ -987,7 +987,7 @@ void DiscoveryDataBase::create_readers_from_change_(
}
else
{
logError(DISCOVERY_DATABASE, "Writer " << reader_guid << " as no associated participant. Skipping");
logError(DISCOVERY_DATABASE, "Reader " << reader_guid << " has no associated participant. Skipping");
return;
}

Expand Down Expand Up @@ -1723,14 +1723,14 @@ void DiscoveryDataBase::AckedFunctor::operator () (
{
// If the participant is already in the DB it means it has answered to the pinging
// or that is pinging us and we have already received its DATA(p)
// If neither of both has happenned we should not wait for it to ack this data, so we
// If either of both has happenned we should not wait for it to ack this data, so we
// skip it and leave it as acked
auto remote_server_it = db_->participants_.find(*it);
if (remote_server_it == db_->participants_.end())
if (remote_server_it != db_->participants_.end())
{
logInfo(DISCOVERY_DATABASE, "Change " << change_->instanceHandle <<
"check as acked for " << reader_proxy->guid() << " as it has not answered pinging yet");
return;
remote_server_it->second.add_or_update_ack_participant(db_->server_guid_prefix_, true);
}
}
}
Expand All @@ -1755,25 +1755,20 @@ void DiscoveryDataBase::unmatch_participant_(
"Attempting to unmatch an unexisting participant: " << guid_prefix);
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this find is not necessary


// For each relevant participant make not relevant
for (eprosima::fastrtps::rtps::GuidPrefix_t relevant_participant : pit->second.relevant_participants())
// For each participant remove it
// IMPORTANT: This is not for every relevant participant, as participant A could be in other participant's B info
// and B not be relevant for A. So it must be done for every Participant.
for (auto& participant_it : participants_)
{
if (relevant_participant != guid_prefix)
{
auto rpit = participants_.find(relevant_participant);
if (rpit == participants_.end())
{
// This is not an error. Remote participants will try to unmatch with participants even
// when the match is not reciprocal
logInfo(DISCOVERY_DATABASE,
"Participant " << relevant_participant << " matched with an unexisting participant: " <<
guid_prefix);
}
else
{
rpit->second.remove_participant(guid_prefix);
}
}
participant_it.second.remove_participant(guid_prefix);
}
for (auto& writer_it : writers_)
{
writer_it.second.remove_participant(guid_prefix);
}
for (auto& reader_it : readers_)
{
reader_it.second.remove_participant(guid_prefix);
}
}

Expand Down
26 changes: 26 additions & 0 deletions src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -801,6 +801,32 @@ bool get_server_client_default_guidPrefix(
return false;
}

bool PDPClient::remove_remote_participant(
const GUID_t& partGUID,
ParticipantDiscoveryInfo::DISCOVERY_STATUS reason)
{
if (PDP::remove_remote_participant(partGUID, reason))
{
// If it works fine, return
return true;
}

// Erase Proxies created before having the Participant
GUID_t wguid;
wguid.guidPrefix = partGUID.guidPrefix;
wguid.entityId = c_EntityId_SPDPWriter;
mp_PDPReader->matched_writer_remove(wguid);

GUID_t rguid;
rguid.guidPrefix = partGUID.guidPrefix;
rguid.entityId = c_EntityId_SPDPReader;
mp_PDPWriter->matched_reader_remove(rguid);

update_remote_servers_list();

return false;
}

} /* namespace rtps */
} /* namespace fastdds */
} /* namespace eprosima */
10 changes: 10 additions & 0 deletions src/cpp/rtps/builtin/discovery/participant/PDPClient.h
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,16 @@ class PDPClient : public PDP
void notifyAboveRemoteEndpoints(
const ParticipantProxyData& pdata) override;

/**
* This method removes a remote RTPSParticipant and all its writers and readers.
* @param participant_guid GUID_t of the remote RTPSParticipant.
* @param reason Why the participant is being removed (dropped vs removed)
* @return true if correct.
*/
bool remove_remote_participant(
const GUID_t& participant_guid,
ParticipantDiscoveryInfo::DISCOVERY_STATUS reason) override;

/**
* Matching server EDP endpoints
* @return true if all servers have been discovered
Expand Down
4 changes: 4 additions & 0 deletions src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1255,6 +1255,10 @@ bool PDPServer::process_to_send_lists()
logInfo(RTPS_PDP_SERVER, "Processing pdp_to_send");
process_to_send_list(discovery_db_.pdp_to_send(), mp_PDPWriter, mp_PDPWriterHistory);
}
else if (!discovery_db_.server_acked_by_all())
{
announceParticipantState(false);
}
else
{
logInfo(RTPS_PDP_SERVER, "Skiping sending PDP data because no entities have been discovered or updated");
Expand Down