From bdc70892c647f0d7672aba413100730819a4b217 Mon Sep 17 00:00:00 2001 From: Attila Molnar Date: Sun, 25 Jan 2015 13:41:24 +0100 Subject: [PATCH] m_spanningtree Rewrite PING logic to use Timers --- src/modules/m_spanningtree/main.cpp | 59 ------------- src/modules/m_spanningtree/main.h | 4 - src/modules/m_spanningtree/pingtimer.cpp | 102 ++++++++++++++++++++++ src/modules/m_spanningtree/pingtimer.h | 77 ++++++++++++++++ src/modules/m_spanningtree/pong.cpp | 4 +- src/modules/m_spanningtree/treeserver.cpp | 36 ++------ src/modules/m_spanningtree/treeserver.h | 33 ++----- 7 files changed, 195 insertions(+), 120 deletions(-) create mode 100644 src/modules/m_spanningtree/pingtimer.cpp create mode 100644 src/modules/m_spanningtree/pingtimer.h diff --git a/src/modules/m_spanningtree/main.cpp b/src/modules/m_spanningtree/main.cpp index c21064683..31d822789 100644 --- a/src/modules/m_spanningtree/main.cpp +++ b/src/modules/m_spanningtree/main.cpp @@ -153,64 +153,6 @@ std::string ModuleSpanningTree::TimeToStr(time_t secs) + ConvToStr(secs) + "s"); } -void ModuleSpanningTree::DoPingChecks(time_t curtime) -{ - /* - * Cancel remote burst mode on any servers which still have it enabled due to latency/lack of data. - * This prevents lost REMOTECONNECT notices - */ - long ts = ServerInstance->Time() * 1000 + (ServerInstance->Time_ns() / 1000000); - -restart: - for (server_hash::iterator i = Utils->serverlist.begin(); i != Utils->serverlist.end(); i++) - { - TreeServer *s = i->second; - - // Skip myself - if (s->IsRoot()) - continue; - - // Do not ping servers that are not fully connected yet! - // Servers which are connected to us have IsLocal() == true and if they're fully connected - // then Socket->LinkState == CONNECTED. Servers that are linked to another server are always fully connected. - if (s->IsLocal() && s->GetSocket()->GetLinkState() != CONNECTED) - continue; - - // Now do PING checks on all servers - // Only ping if this server needs one - if (curtime >= s->NextPingTime()) - { - // And if they answered the last - if (s->AnsweredLastPing()) - { - // They did, send a ping to them - s->SetNextPingTime(curtime + Utils->PingFreq); - s->GetSocket()->WriteLine(CmdBuilder("PING").push(s->GetID())); - s->LastPingMsec = ts; - } - else - { - // They didn't answer the last ping, if they are locally connected, get rid of them. - if (s->IsLocal()) - { - TreeSocket* sock = s->GetSocket(); - sock->SendError("Ping timeout"); - sock->Close(); - goto restart; - } - } - } - - // If warn on ping enabled and not warned and the difference is sufficient and they didn't answer the last ping... - if ((Utils->PingWarnTime) && (!s->Warned) && (curtime >= s->NextPingTime() - (Utils->PingFreq - Utils->PingWarnTime)) && (!s->AnsweredLastPing())) - { - /* The server hasnt responded, send a warning to opers */ - ServerInstance->SNO->WriteToSnoMask('l',"Server \002%s\002 has not responded to PING for %d seconds, high latency.", s->GetName().c_str(), Utils->PingWarnTime); - s->Warned = true; - } - } -} - void ModuleSpanningTree::ConnectServer(Autoconnect* a, bool on_timer) { if (!a) @@ -471,7 +413,6 @@ void ModuleSpanningTree::OnUserMessage(User* user, void* dest, int target_type, void ModuleSpanningTree::OnBackgroundTimer(time_t curtime) { AutoConnectServers(curtime); - DoPingChecks(curtime); DoConnectTimeout(curtime); } diff --git a/src/modules/m_spanningtree/main.h b/src/modules/m_spanningtree/main.h index c81eb2d0b..13c743f73 100644 --- a/src/modules/m_spanningtree/main.h +++ b/src/modules/m_spanningtree/main.h @@ -103,10 +103,6 @@ class ModuleSpanningTree : public Module */ ModResult HandleRemoteWhois(const std::vector& parameters, User* user); - /** Ping all local servers - */ - void DoPingChecks(time_t curtime); - /** Connect a server locally */ void ConnectServer(Link* x, Autoconnect* y = NULL); diff --git a/src/modules/m_spanningtree/pingtimer.cpp b/src/modules/m_spanningtree/pingtimer.cpp new file mode 100644 index 000000000..1c96259bf --- /dev/null +++ b/src/modules/m_spanningtree/pingtimer.cpp @@ -0,0 +1,102 @@ +/* + * InspIRCd -- Internet Relay Chat Daemon + * + * Copyright (C) 2015 Attila Molnar + * + * This file is part of InspIRCd. InspIRCd is free software: you can + * redistribute it and/or modify it under the terms of the GNU General Public + * License as published by the Free Software Foundation, version 2. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + * FOR A PARTICULAR PURPOSE. See the GNU General Public License for more + * details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + + +#include "inspircd.h" + +#include "pingtimer.h" +#include "treeserver.h" +#include "commandbuilder.h" + +PingTimer::PingTimer(TreeServer* ts) + : Timer(Utils->PingFreq) + , server(ts) + , state(PS_SENDPING) +{ +} + +PingTimer::State PingTimer::TickInternal() +{ + // Timer expired, take next action based on what happened last time + if (state == PS_SENDPING) + { + // Last ping was answered, send next ping + server->GetSocket()->WriteLine(CmdBuilder("PING").push(server->GetID())); + LastPingMsec = ServerInstance->Time() * 1000 + (ServerInstance->Time_ns() / 1000000); + // Warn next unless warnings are disabled. If they are, jump straight to timeout. + if (Utils->PingWarnTime) + return PS_WARN; + else + return PS_TIMEOUT; + } + else if (state == PS_WARN) + { + // No pong arrived in PingWarnTime seconds, send a warning to opers + ServerInstance->SNO->WriteToSnoMask('l', "Server \002%s\002 has not responded to PING for %d seconds, high latency.", server->GetName().c_str(), GetInterval()); + return PS_TIMEOUT; + } + else // PS_TIMEOUT + { + // They didn't answer the last ping, if they are locally connected, get rid of them + if (server->IsLocal()) + { + TreeSocket* sock = server->GetSocket(); + sock->SendError("Ping timeout"); + sock->Close(); + } + + // If the server is non-locally connected, don't do anything until we get a PONG. + // This is to avoid pinging the server and warning opers more than once. + // If they do answer eventually, we will move to the PS_SENDPING state and ping them again. + return PS_IDLE; + } +} + +void PingTimer::SetState(State newstate) +{ + state = newstate; + + // Set when should the next Tick() happen based on the state + if (state == PS_SENDPING) + SetInterval(Utils->PingFreq); + else if (state == PS_WARN) + SetInterval(Utils->PingWarnTime); + else if (state == PS_TIMEOUT) + SetInterval(Utils->PingFreq - Utils->PingWarnTime); + + // If state == PS_IDLE, do not set the timer, see above why +} + +bool PingTimer::Tick(time_t currtime) +{ + if (server->IsDead()) + return false; + + SetState(TickInternal()); + return false; +} + +void PingTimer::OnPong() +{ + // Calculate RTT + long ts = ServerInstance->Time() * 1000 + (ServerInstance->Time_ns() / 1000000); + server->rtt = ts - LastPingMsec; + + // Change state to send ping next, also reschedules the timer appropriately + SetState(PS_SENDPING); +} diff --git a/src/modules/m_spanningtree/pingtimer.h b/src/modules/m_spanningtree/pingtimer.h new file mode 100644 index 000000000..753558689 --- /dev/null +++ b/src/modules/m_spanningtree/pingtimer.h @@ -0,0 +1,77 @@ +/* + * InspIRCd -- Internet Relay Chat Daemon + * + * Copyright (C) 2015 Attila Molnar + * + * This file is part of InspIRCd. InspIRCd is free software: you can + * redistribute it and/or modify it under the terms of the GNU General Public + * License as published by the Free Software Foundation, version 2. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + * FOR A PARTICULAR PURPOSE. See the GNU General Public License for more + * details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + + +#pragma once + +class TreeServer; + +/** Handles PINGing servers and killing them on timeout + */ +class PingTimer : public Timer +{ + enum State + { + /** Send PING next */ + PS_SENDPING, + /** Warn opers next */ + PS_WARN, + /** Kill the server next due to ping timeout */ + PS_TIMEOUT, + /** Do nothing */ + PS_IDLE + }; + + /** Server the timer is interacting with + */ + TreeServer* const server; + + /** What to do when the timer ticks next + */ + State state; + + /** Last ping time in milliseconds, used to calculate round trip time + */ + unsigned long LastPingMsec; + + /** Update internal state and reschedule timer according to the new state + * @param newstate State to change to + */ + void SetState(State newstate); + + /** Process timer tick event + * @return State to change to + */ + State TickInternal(); + + /** Called by the TimerManager when the timer expires + * @param currtime Time now + * @return Always false, we reschedule ourselves instead + */ + bool Tick(time_t currtime) CXX11_OVERRIDE; + + public: + /** Construct the timer. This doesn't schedule the timer. + * @param server TreeServer to interact with + */ + PingTimer(TreeServer* server); + + /** Register a PONG from the server + */ + void OnPong(); +}; diff --git a/src/modules/m_spanningtree/pong.cpp b/src/modules/m_spanningtree/pong.cpp index a7dc64f83..5d97f2af2 100644 --- a/src/modules/m_spanningtree/pong.cpp +++ b/src/modules/m_spanningtree/pong.cpp @@ -35,9 +35,7 @@ CmdResult CommandPong::HandleServer(TreeServer* server, std::vector if (params[0] == ServerInstance->Config->GetSID()) { // PONG for us - long ts = ServerInstance->Time() * 1000 + (ServerInstance->Time_ns() / 1000000); - server->rtt = ts - server->LastPingMsec; - server->SetPingFlag(); + server->OnPong(); } return CMD_SUCCESS; } diff --git a/src/modules/m_spanningtree/treeserver.cpp b/src/modules/m_spanningtree/treeserver.cpp index 534315ff7..e004f897e 100644 --- a/src/modules/m_spanningtree/treeserver.cpp +++ b/src/modules/m_spanningtree/treeserver.cpp @@ -38,8 +38,9 @@ TreeServer::TreeServer() , VersionString(ServerInstance->GetVersionString()) , fullversion(ServerInstance->GetVersionString(true)) , Socket(NULL), sid(ServerInstance->Config->GetSID()), behind_bursting(0), isdead(false) + , pingtimer(this) , ServerUser(ServerInstance->FakeClient) - , age(ServerInstance->Time()), Warned(false), UserCount(ServerInstance->Users.GetLocalUsers().size()) + , age(ServerInstance->Time()), UserCount(ServerInstance->Users.GetLocalUsers().size()) , OperCount(0), rtt(0), StartBurst(0), Hidden(false) { AddHashEntry(); @@ -52,13 +53,14 @@ TreeServer::TreeServer() TreeServer::TreeServer(const std::string& Name, const std::string& Desc, const std::string& id, TreeServer* Above, TreeSocket* Sock, bool Hide) : Server(Name, Desc) , Parent(Above), Socket(Sock), sid(id), behind_bursting(Parent->behind_bursting), isdead(false) + , pingtimer(this) , ServerUser(new FakeUser(id, this)) - , age(ServerInstance->Time()), Warned(false), UserCount(0), OperCount(0), rtt(0), StartBurst(0), Hidden(Hide) + , age(ServerInstance->Time()), UserCount(0), OperCount(0), rtt(0), StartBurst(0), Hidden(Hide) { ServerInstance->Logs->Log(MODNAME, LOG_DEBUG, "New server %s behind_bursting %u", GetName().c_str(), behind_bursting); CheckULine(); - SetNextPingTime(ServerInstance->Time() + Utils->PingFreq); - SetPingFlag(); + + ServerInstance->Timers.AddTimer(&pingtimer); /* find the 'route' for this server (e.g. the one directly connected * to the local server, which we can use to reach it) @@ -135,11 +137,6 @@ void TreeServer::FinishBurstInternal() behind_bursting--; ServerInstance->Logs->Log(MODNAME, LOG_DEBUG, "FinishBurstInternal() %s behind_bursting %u", GetName().c_str(), behind_bursting); - if (!IsBehindBursting()) - { - SetNextPingTime(ServerInstance->Time() + Utils->PingFreq); - SetPingFlag(); - } for (ChildServers::const_iterator i = Children.begin(); i != Children.end(); ++i) { TreeServer* child = *i; @@ -261,27 +258,6 @@ void TreeServer::AddHashEntry() Utils->sidlist[sid] = this; } -void TreeServer::SetNextPingTime(time_t t) -{ - this->NextPing = t; - LastPingWasGood = false; -} - -time_t TreeServer::NextPingTime() -{ - return NextPing; -} - -bool TreeServer::AnsweredLastPing() -{ - return LastPingWasGood; -} - -void TreeServer::SetPingFlag() -{ - LastPingWasGood = true; -} - CullResult TreeServer::cull() { // Recursively cull all servers that are under us in the tree diff --git a/src/modules/m_spanningtree/treeserver.h b/src/modules/m_spanningtree/treeserver.h index 4465de15e..1a0203ba0 100644 --- a/src/modules/m_spanningtree/treeserver.h +++ b/src/modules/m_spanningtree/treeserver.h @@ -22,6 +22,7 @@ #pragma once #include "treesocket.h" +#include "pingtimer.h" /** Each server in the tree is represented by one class of * type TreeServer. A locally connected TreeServer can @@ -49,8 +50,6 @@ class TreeServer : public Server std::string fullversion; TreeSocket* Socket; /* Socket used to communicate with this server */ - time_t NextPing; /* After this time, the server should be PINGed*/ - bool LastPingWasGood; /* True if the server responded to the last PING with a PONG */ std::string sid; /* Server ID */ /** Counter counting how many servers are bursting in front of this server, including @@ -64,6 +63,10 @@ class TreeServer : public Server */ bool isdead; + /** Timer handling PINGing the server and killing it on timeout + */ + PingTimer pingtimer; + /** This method is used to add this TreeServer to the * hash maps. It is only called by the constructors. */ @@ -82,8 +85,6 @@ class TreeServer : public Server FakeUser* const ServerUser; /* User representing this server */ const time_t age; - bool Warned; /* True if we've warned opers about high latency on this server */ - unsigned int UserCount; /* How many users are on this server? [note: doesn't care about +i] */ unsigned int OperCount; /* How many opers are on this server? */ @@ -143,18 +144,6 @@ class TreeServer : public Server */ const std::string& GetFullVersion() const { return fullversion; } - /** Set time we are next due to ping this server - */ - void SetNextPingTime(time_t t); - - /** Get the time we are next due to ping this server - */ - time_t NextPingTime(); - - /** Last ping time in milliseconds, used to calculate round trip time - */ - unsigned long LastPingMsec; - /** Round trip time of last ping */ unsigned long rtt; @@ -167,14 +156,6 @@ class TreeServer : public Server */ bool Hidden; - /** True if the server answered their last ping - */ - bool AnsweredLastPing(); - - /** Set the server as responding to its last ping - */ - void SetPingFlag(); - /** Get the TreeSocket pointer for local servers. * For remote servers, this returns NULL. */ @@ -234,6 +215,10 @@ class TreeServer : public Server */ void BeginBurst(unsigned long startms = 0); + /** Register a PONG from the server + */ + void OnPong() { pingtimer.OnPong(); } + CullResult cull(); /** Destructor, deletes ServerUser unless IsRoot() -- 2.39.5