From 2e24d0bfa7c8329f56607b0c7d041a61542d344a Mon Sep 17 00:00:00 2001 From: Eduard Bagdasaryan Date: Thu, 31 Aug 2023 23:00:01 +0000 Subject: [PATCH] Do not use invasive lists to store CachePeers (#1424) Using invasive lists for CachePeer objects gives no advantages, but requires maintaining non-standard error-prone code that leads to excessive locking and associated memory overheads. Also fixed a neighbors_init() bug that resulted in accessing an already deleted "looks like this host" CachePeer object. --- src/CachePeer.cc | 2 - src/CachePeer.h | 2 +- src/CachePeers.cc | 56 ++++++++++++++++ src/CachePeers.h | 43 ++++++++++++ src/Makefile.am | 7 ++ src/PeerPoolMgr.cc | 4 +- src/SquidConfig.h | 5 +- src/cache_cf.cc | 30 +++++---- src/carp.cc | 4 +- src/htcp.cc | 5 +- src/icmp/net_db.cc | 4 +- src/neighbors.cc | 148 +++++++++++++---------------------------- src/neighbors.h | 2 - src/peer_select.cc | 7 +- src/peer_sourcehash.cc | 17 ++--- src/peer_userhash.cc | 17 ++--- src/snmp_agent.cc | 9 +-- src/snmp_core.cc | 10 +-- src/stat.cc | 4 +- 19 files changed, 217 insertions(+), 159 deletions(-) create mode 100644 src/CachePeers.cc diff --git a/src/CachePeer.cc b/src/CachePeer.cc index 01db8ab24e..63c6fb60a9 100644 --- a/src/CachePeer.cc +++ b/src/CachePeer.cc @@ -47,8 +47,6 @@ CachePeer::~CachePeer() xfree(digest_url); #endif - delete next; - xfree(login); delete standby.pool; diff --git a/src/CachePeer.h b/src/CachePeer.h index 2f3ff30af5..e579a9c322 100644 --- a/src/CachePeer.h +++ b/src/CachePeer.h @@ -48,6 +48,7 @@ public: /// \returns the effective connect timeout for the given peer time_t connectTimeout() const; + /// n-th cache_peer directive, starting with 1 u_int index = 0; /// cache_peer name (if explicitly configured) or hostname (otherwise). @@ -179,7 +180,6 @@ public: Ip::Address addresses[10]; int n_addresses = 0; int rr_count = 0; - CachePeer *next = nullptr; int testing_now = 0; struct { diff --git a/src/CachePeers.cc b/src/CachePeers.cc new file mode 100644 index 0000000000..8ebac56bba --- /dev/null +++ b/src/CachePeers.cc @@ -0,0 +1,56 @@ +/* + * Copyright (C) 1996-2023 The Squid Software Foundation and contributors + * + * Squid software is distributed under GPLv2+ license and includes + * contributions from numerous individuals and organizations. + * Please see the COPYING and CONTRIBUTORS files for details. + */ + +#include "squid.h" +#include "CachePeers.h" +#include "SquidConfig.h" + +CachePeer & +CachePeers::nextPeerToPing(const size_t pollIndex) +{ + Assure(size()); + + // Remember the number of polls to keep shifting each poll starting point, + // to avoid always polling the same group of peers before other peers and + // risk overloading that first group with requests. + if (!pollIndex) + ++peerPolls_; + + // subtract 1 to set the very first pos to zero + const auto pos = (peerPolls_ - 1 + pollIndex) % size(); + + return *storage[pos]; +} + +void +CachePeers::remove(CachePeer * const peer) +{ + const auto pos = std::find_if(storage.begin(), storage.end(), [&](const auto &storePeer) { + return storePeer.get() == peer; + }); + Assure(pos != storage.end()); + storage.erase(pos); +} + +const CachePeers & +CurrentCachePeers() +{ + if (Config.peers) + return *Config.peers; + + static const CachePeers empty; + return empty; +} + +void +DeleteConfigured(CachePeer * const peer) +{ + Assure(Config.peers); + Config.peers->remove(peer); +} + diff --git a/src/CachePeers.h b/src/CachePeers.h index e15d6d0e52..6d915c4a27 100644 --- a/src/CachePeers.h +++ b/src/CachePeers.h @@ -13,8 +13,51 @@ #include "CachePeer.h" #include "mem/PoolingAllocator.h" +#include #include +/// cache_peer configuration storage +class CachePeers +{ +public: + /// owns stored CachePeer objects + using Storage = std::vector< std::unique_ptr, PoolingAllocator< std::unique_ptr > >; + + /// stores a being-configured cache_peer + void add(CachePeer *p) { storage.emplace_back(p); } + + /// deletes a previously add()ed CachePeer object + void remove(CachePeer *); + + /// the number of currently stored (i.e. added and not removed) cache_peers + auto size() const { return storage.size(); } + + /* peer iterators forming a sequence for C++ range-based for loop API */ + using const_iterator = Storage::const_iterator; + auto begin() const { return storage.cbegin(); } + auto end() const { return storage.cend(); } + + /// A CachePeer to query next when scanning all peer caches in hope to fetch + /// a remote cache hit. \sa neighborsUdpPing() + /// \param iteration a 0-based index of a loop scanning all peers + CachePeer &nextPeerToPing(size_t iteration); + +private: + /// cache_peers in configuration/parsing order + Storage storage; + + /// total number of completed peer scans by nextPeerToPing()-calling code + uint64_t peerPolls_ = 0; +}; + +/// All configured cache_peers that are still available/relevant. +/// \returns an empty container if no cache_peers were configured or all +/// configured cache_peers were removed (e.g., by DeleteConfigured()). +const CachePeers &CurrentCachePeers(); + +/// destroys the given peer after removing it from the set of configured peers +void DeleteConfigured(CachePeer *); + /// Weak pointers to zero or more Config.peers. /// Users must specify the selection algorithm and the order of entries. using SelectedCachePeers = std::vector< CbcPointer, PoolingAllocator< CbcPointer > >; diff --git a/src/Makefile.am b/src/Makefile.am index f2eee16555..0e98936913 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -199,6 +199,7 @@ squid_SOURCES = \ CacheManager.h \ CachePeer.cc \ CachePeer.h \ + CachePeers.cc \ CachePeers.h \ ClientInfo.h \ ClientRequestContext.h \ @@ -1741,6 +1742,8 @@ tests_test_http_range_SOURCES = \ CacheDigest.h \ CachePeer.cc \ CachePeer.h \ + CachePeers.cc \ + CachePeers.h \ ClientInfo.h \ tests/stub_CollapsedForwarding.cc \ ConfigOption.cc \ @@ -2124,6 +2127,8 @@ tests_testHttpRequest_SOURCES = \ CacheDigest.h \ CachePeer.cc \ CachePeer.h \ + CachePeers.cc \ + CachePeers.h \ ClientInfo.h \ tests/stub_CollapsedForwarding.cc \ ConfigOption.cc \ @@ -2420,6 +2425,8 @@ tests_testCacheManager_SOURCES = \ tests/testCacheManager.cc \ CachePeer.cc \ CachePeer.h \ + CachePeers.cc \ + CachePeers.h \ ClientInfo.h \ tests/stub_CollapsedForwarding.cc \ ConfigOption.cc \ diff --git a/src/PeerPoolMgr.cc b/src/PeerPoolMgr.cc index 7becaf636d..6b6b440b6b 100644 --- a/src/PeerPoolMgr.cc +++ b/src/PeerPoolMgr.cc @@ -11,6 +11,7 @@ #include "base/AsyncCallbacks.h" #include "base/RunnersRegistry.h" #include "CachePeer.h" +#include "CachePeers.h" #include "comm/Connection.h" #include "comm/ConnOpener.h" #include "debug/Stream.h" @@ -244,7 +245,8 @@ DefineRunnerRegistrator(PeerPoolMgrsRr); void PeerPoolMgrsRr::syncConfig() { - for (CachePeer *p = Config.peers; p; p = p->next) { + for (const auto &peer: CurrentCachePeers()) { + const auto p = peer.get(); // On reconfigure, Squid deletes the old config (and old peers in it), // so should always be dealing with a brand new configuration. assert(!p->standby.mgr); diff --git a/src/SquidConfig.h b/src/SquidConfig.h index 7cb3bbedb2..d9ec3d0935 100644 --- a/src/SquidConfig.h +++ b/src/SquidConfig.h @@ -42,7 +42,8 @@ namespace Mgr { class ActionPasswordList; } // namespace Mgr -class CachePeer; + +class CachePeers; class CustomLog; class CpuAffinityMap; class DebugMessages; @@ -242,7 +243,7 @@ public: size_t tcpRcvBufsz; size_t udpMaxHitObjsz; wordlist *mcast_group_list; - CachePeer *peers; + CachePeers *peers; int npeers; struct { diff --git a/src/cache_cf.cc b/src/cache_cf.cc index cb1db1fcc5..3437a31541 100644 --- a/src/cache_cf.cc +++ b/src/cache_cf.cc @@ -25,6 +25,7 @@ #include "base/RunnersRegistry.h" #include "cache_cf.h" #include "CachePeer.h" +#include "CachePeers.h" #include "ConfigOption.h" #include "ConfigParser.h" #include "CpuAffinityMap.h" @@ -975,7 +976,7 @@ configDoConfigure(void) #endif } - for (CachePeer *p = Config.peers; p != nullptr; p = p->next) { + for (const auto &p: CurrentCachePeers()) { // default value for ssldomain= is the peer host/IP if (p->secure.sslDomain.isEmpty()) @@ -2068,12 +2069,16 @@ peer_type_str(const peer_t type) } static void -dump_peer(StoreEntry * entry, const char *name, CachePeer * p) +dump_peer(StoreEntry * entry, const char *name, const CachePeers *peers) { + if (!peers) + return; + NeighborTypeDomainList *t; LOCAL_ARRAY(char, xname, 128); - while (p != nullptr) { + for (const auto &peer: *peers) { + const auto p = peer.get(); storeAppendPrintf(entry, "%s %s %s %d %d name=%s", name, p->host, @@ -2094,8 +2099,6 @@ dump_peer(StoreEntry * entry, const char *name, CachePeer * p) peer_type_str(t->type), t->domain); } - - p = p->next; } } @@ -2160,7 +2163,7 @@ GetUdpService(void) } static void -parse_peer(CachePeer ** head) +parse_peer(CachePeers **peers) { char *host_str = ConfigParser::NextToken(); if (!host_str) { @@ -2397,22 +2400,21 @@ parse_peer(CachePeer ** head) if (p->secure.encryptTransport) p->secure.parseOptions(); - p->index = ++Config.npeers; + if (!*peers) + *peers = new CachePeers; - while (*head != nullptr) - head = &(*head)->next; + (*peers)->add(p); - *head = p; + p->index = (*peers)->size(); peerClearRRStart(); } static void -free_peer(CachePeer ** P) +free_peer(CachePeers ** const peers) { - delete *P; - *P = nullptr; - Config.npeers = 0; + delete *peers; + *peers = nullptr; } static void diff --git a/src/carp.cc b/src/carp.cc index 7d747d2b3a..d8f1738945 100644 --- a/src/carp.cc +++ b/src/carp.cc @@ -58,7 +58,9 @@ carpInit(void) /* find out which peers we have */ RawCachePeers rawCarpPeers; - for (auto p = Config.peers; p; p = p->next) { + for (const auto &peer: CurrentCachePeers()) { + const auto p = peer.get(); + if (!p->options.carp) continue; diff --git a/src/htcp.cc b/src/htcp.cc index 2296a40d26..21cdf43c6f 100644 --- a/src/htcp.cc +++ b/src/htcp.cc @@ -14,6 +14,7 @@ #include "acl/FilledChecklist.h" #include "base/AsyncCallbacks.h" #include "CachePeer.h" +#include "CachePeers.h" #include "comm.h" #include "comm/Connection.h" #include "comm/Loops.h" @@ -1272,9 +1273,7 @@ htcpHandleClr(htcpDataHeader * hdr, char *buf, int sz, Ip::Address &from) static void htcpForwardClr(char *buf, int sz) { - CachePeer *p; - - for (p = Config.peers; p; p = p->next) { + for (const auto &p: CurrentCachePeers()) { if (!p->options.htcp) { continue; } diff --git a/src/icmp/net_db.cc b/src/icmp/net_db.cc index 46dd0b009e..664fffefd7 100644 --- a/src/icmp/net_db.cc +++ b/src/icmp/net_db.cc @@ -18,6 +18,7 @@ #include "squid.h" #include "CachePeer.h" +#include "CachePeers.h" #include "cbdata.h" #include "event.h" #include "fde.h" @@ -1233,7 +1234,8 @@ netdbExchangeStart(void *data) static CachePeer * findUsableParentAtHostname(PeerSelector *ps, const char * const hostname, const HttpRequest &request) { - for (auto p = Config.peers; p; p = p->next) { + for (const auto &peer: CurrentCachePeers()) { + const auto p = peer.get(); // Both fields should be lowercase, but no code ensures that invariant. // TODO: net_db_peer should point to CachePeer instead of its hostname! if (strcasecmp(p->host, hostname) != 0) diff --git a/src/neighbors.cc b/src/neighbors.cc index 194745a913..9ca8bdfca9 100644 --- a/src/neighbors.cc +++ b/src/neighbors.cc @@ -16,6 +16,7 @@ #include "base/PackableStream.h" #include "CacheDigest.h" #include "CachePeer.h" +#include "CachePeers.h" #include "comm/Connection.h" #include "comm/ConnOpener.h" #include "debug/Messages.h" @@ -52,7 +53,6 @@ bool peerAllowedToUse(const CachePeer *, PeerSelector *); static int peerWouldBePinged(const CachePeer *, PeerSelector *); -static void neighborRemove(CachePeer *); static void neighborAlive(CachePeer *, const MemObject *, const icp_common_t *); #if USE_HTCP static void neighborAliveHtcp(CachePeer *, const MemObject *, const HtcpReplyData *); @@ -71,12 +71,11 @@ static IRCB peerCountHandleIcpReply; static void neighborIgnoreNonPeer(const Ip::Address &, icp_opcode); static OBJH neighborDumpPeers; -static void dump_peers(StoreEntry * sentry, CachePeer * peers); +static void dump_peers(StoreEntry *, CachePeers *); static unsigned short echo_port; static int NLateReplies = 0; -static CachePeer *first_ping = nullptr; const char * neighborTypeStr(const CachePeer * p) @@ -98,13 +97,12 @@ whichPeer(const Ip::Address &from) { int j; - CachePeer *p = nullptr; debugs(15, 3, "whichPeer: from " << from); - for (p = Config.peers; p; p = p->next) { + for (const auto &p: CurrentCachePeers()) { for (j = 0; j < p->n_addresses; ++j) { if (from == p->addresses[j] && from.port() == p->icp.port) { - return p; + return p.get(); } } } @@ -273,11 +271,10 @@ peerHTTPOkay(const CachePeer * p, PeerSelector * ps) int neighborsCount(PeerSelector *ps) { - CachePeer *p = nullptr; int count = 0; - for (p = Config.peers; p; p = p->next) - if (peerWouldBePinged(p, ps)) + for (const auto &p: CurrentCachePeers()) + if (peerWouldBePinged(p.get(), ps)) ++count; debugs(15, 3, "neighborsCount: " << count); @@ -293,7 +290,9 @@ getFirstUpParent(PeerSelector *ps) CachePeer *p = nullptr; - for (p = Config.peers; p; p = p->next) { + for (const auto &peer: CurrentCachePeers()) { + p = peer.get(); + if (!neighborUp(p)) continue; @@ -316,10 +315,10 @@ getRoundRobinParent(PeerSelector *ps) assert(ps); HttpRequest *request = ps->request; - CachePeer *p; CachePeer *q = nullptr; - for (p = Config.peers; p; p = p->next) { + for (const auto &peer: CurrentCachePeers()) { + const auto p = peer.get(); if (!p->options.roundrobin) continue; @@ -358,11 +357,12 @@ getWeightedRoundRobinParent(PeerSelector *ps) assert(ps); HttpRequest *request = ps->request; - CachePeer *p; CachePeer *q = nullptr; int weighted_rtt; - for (p = Config.peers; p; p = p->next) { + for (const auto &peer: CurrentCachePeers()) { + const auto p = peer.get(); + if (!p->options.weighted_roundrobin) continue; @@ -379,11 +379,11 @@ getWeightedRoundRobinParent(PeerSelector *ps) } if (q && q->rr_count > 1000000) - for (p = Config.peers; p; p = p->next) { + for (const auto &p: CurrentCachePeers()) { if (!p->options.weighted_roundrobin) continue; - if (neighborType(p, request->url) != PEER_PARENT) + if (neighborType(p.get(), request->url) != PEER_PARENT) continue; p->rr_count = 0; @@ -447,10 +447,8 @@ peerClearRRStart(void) void peerClearRR() { - CachePeer *p = nullptr; - for (p = Config.peers; p; p = p->next) { + for (const auto &p: CurrentCachePeers()) p->rr_count = 1; - } } void @@ -478,9 +476,9 @@ getDefaultParent(PeerSelector *ps) assert(ps); HttpRequest *request = ps->request; - CachePeer *p = nullptr; + for (const auto &peer: CurrentCachePeers()) { + const auto p = peer.get(); - for (p = Config.peers; p; p = p->next) { if (neighborType(p, request->url) != PEER_PARENT) continue; @@ -500,45 +498,6 @@ getDefaultParent(PeerSelector *ps) return nullptr; } -CachePeer * -getNextPeer(CachePeer * p) -{ - return p->next; -} - -CachePeer * -getFirstPeer(void) -{ - return Config.peers; -} - -static void -neighborRemove(CachePeer * target) -{ - CachePeer *p = nullptr; - CachePeer **P = nullptr; - p = Config.peers; - P = &Config.peers; - - while (p) { - if (target == p) - break; - - P = &p->next; - - p = p->next; - } - - if (p) { - *P = p->next; - p->next = nullptr; - delete p; - --Config.npeers; - } - - first_ping = Config.peers; -} - static void neighborsRegisterWithCacheManager() { @@ -552,16 +511,13 @@ neighbors_init(void) { struct servent *sep = nullptr; const char *me = getMyHostname(); - CachePeer *thisPeer = nullptr; - CachePeer *next = nullptr; neighborsRegisterWithCacheManager(); if (Comm::IsConnOpen(icpIncomingConn)) { + RawCachePeers peersToRemove; - for (thisPeer = Config.peers; thisPeer; thisPeer = next) { - next = thisPeer->next; - + for (const auto &thisPeer: CurrentCachePeers()) { if (0 != strcmp(thisPeer->host, me)) continue; @@ -572,17 +528,22 @@ neighbors_init(void) debugs(15, DBG_IMPORTANT, "WARNING: Peer looks like this host." << Debug::Extra << "Ignoring cache_peer " << *thisPeer); - neighborRemove(thisPeer); + peersToRemove.push_back(thisPeer.get()); + break; // avoid warning about (and removing) the same CachePeer twice } } + + while (peersToRemove.size()) { + const auto p = peersToRemove.back(); + peersToRemove.pop_back(); + DeleteConfigured(p); + } } peerRefreshDNS((void *) 1); sep = getservbyname("echo", "udp"); echo_port = sep ? ntohs((unsigned short) sep->s_port) : 7; - - first_ping = Config.peers; } int @@ -595,8 +556,6 @@ neighborsUdpPing(HttpRequest * request, { const char *url = entry->url(); MemObject *mem = entry->mem_obj; - CachePeer *p = nullptr; - int i; int reqnum = 0; int flags; int peers_pinged = 0; @@ -617,9 +576,8 @@ neighborsUdpPing(HttpRequest * request, reqnum = icpSetCacheKey((const cache_key *)entry->key); - for (i = 0, p = first_ping; i++ < Config.npeers; p = p->next) { - if (p == nullptr) - p = Config.peers; + for (size_t i = 0; i < Config.peers->size(); ++i) { + const auto p = &Config.peers->nextPeerToPing(i); debugs(15, 5, "candidate: " << *p); @@ -710,9 +668,6 @@ neighborsUdpPing(HttpRequest * request, p->stats.probe_start = squid_curtime; } - if ((first_ping = first_ping->next) == nullptr) - first_ping = Config.peers; - /* * How many replies to expect? */ @@ -803,25 +758,17 @@ neighborsDigestSelect(PeerSelector *ps) int best_rtt = 0; int choice_count = 0; int ichoice_count = 0; - CachePeer *p; int p_rtt; - int i; if (!request->flags.hierarchical) return nullptr; storeKeyPublicByRequest(request); - for (i = 0, p = first_ping; i++ < Config.npeers; p = p->next) { - lookup_t lookup; - - if (!p) - p = Config.peers; + for (size_t i = 0; i < Config.peers->size(); ++i) { + const auto p = &Config.peers->nextPeerToPing(i); - if (i == 1) - first_ping = p; - - lookup = peerDigestLookup(p, ps); + const auto lookup = peerDigestLookup(p, ps); if (lookup == LOOKUP_NONE) continue; @@ -1083,7 +1030,7 @@ neighborsUdpAck(const cache_key * key, icp_common_t * header, const Ip::Address if (100 * p->icp.counts[ICP_DENIED] / p->stats.pings_acked > 95) { debugs(15, DBG_CRITICAL, "Disabling cache_peer " << *p << " because over 95% of its replies are UDP_DENIED"); - neighborRemove(p); + DeleteConfigured(p); p = nullptr; } else { neighborCountIgnored(p); @@ -1099,14 +1046,11 @@ neighborsUdpAck(const cache_key * key, icp_common_t * header, const Ip::Address CachePeer * findCachePeerByName(const char * const name) { - CachePeer *p = nullptr; - - for (p = Config.peers; p; p = p->next) { + for (const auto &p: CurrentCachePeers()) { if (!strcasecmp(name, p->name)) - break; + return p.get(); } - - return p; + return nullptr; } int @@ -1210,8 +1154,6 @@ peerDNSConfigure(const ipcache_addrs *ia, const Dns::LookupDetails &, void *data static void peerRefreshDNS(void *data) { - CachePeer *p = nullptr; - if (eventFind(peerRefreshDNS, nullptr)) eventDelete(peerRefreshDNS, nullptr); @@ -1221,8 +1163,8 @@ peerRefreshDNS(void *data) return; } - for (p = Config.peers; p; p = p->next) - ipcache_nbgethostbyname(p->host, peerDNSConfigure, p); + for (const auto &p: CurrentCachePeers()) + ipcache_nbgethostbyname(p->host, peerDNSConfigure, p.get()); /* Reconfigure the peers every hour */ eventAddIsh("peerRefreshDNS", peerRefreshDNS, nullptr, 3600.0, 1); @@ -1553,7 +1495,7 @@ dump_peer_options(StoreEntry * sentry, CachePeer * p) } static void -dump_peers(StoreEntry * sentry, CachePeer * peers) +dump_peers(StoreEntry *sentry, CachePeers *peers) { char ntoabuf[MAX_IPSTRLEN]; int i; @@ -1561,7 +1503,8 @@ dump_peers(StoreEntry * sentry, CachePeer * peers) if (peers == nullptr) storeAppendPrintf(sentry, "There are no neighbors installed.\n"); - for (CachePeer *e = peers; e; e = e->next) { + for (const auto &peer: *peers) { + const auto e = peer.get(); assert(e->host != nullptr); storeAppendPrintf(sentry, "\n%-11.11s: %s\n", neighborTypeStr(e), @@ -1722,10 +1665,9 @@ neighborsHtcpReply(const cache_key * key, HtcpReplyData * htcp, const Ip::Addres void neighborsHtcpClear(StoreEntry * e, HttpRequest * req, const HttpRequestMethod &method, htcp_clr_reason reason) { - CachePeer *p; char buf[128]; - for (p = Config.peers; p; p = p->next) { + for (const auto &p: CurrentCachePeers()) { if (!p->options.htcp) { continue; } @@ -1736,7 +1678,7 @@ neighborsHtcpClear(StoreEntry * e, HttpRequest * req, const HttpRequestMethod &m continue; } debugs(15, 3, "neighborsHtcpClear: sending CLR to " << p->in_addr.toUrl(buf, 128)); - htcpClear(e, req, method, p, reason); + htcpClear(e, req, method, p.get(), reason); } } diff --git a/src/neighbors.h b/src/neighbors.h index be360139dd..9dd350091d 100644 --- a/src/neighbors.h +++ b/src/neighbors.h @@ -23,9 +23,7 @@ class CachePeer; class StoreEntry; class PeerSelector; -CachePeer *getFirstPeer(void); CachePeer *getFirstUpParent(PeerSelector *); -CachePeer *getNextPeer(CachePeer *); CachePeer *getSingleParent(PeerSelector *); int neighborsCount(PeerSelector *); int neighborsUdpPing(HttpRequest *, diff --git a/src/peer_select.cc b/src/peer_select.cc index 834fb77502..c1e0d89612 100644 --- a/src/peer_select.cc +++ b/src/peer_select.cc @@ -14,6 +14,7 @@ #include "base/InstanceId.h" #include "base/TypeTraits.h" #include "CachePeer.h" +#include "CachePeers.h" #include "carp.h" #include "client_side.h" #include "dns/LookupDetails.h" @@ -865,10 +866,10 @@ PeerSelector::selectSomeParent() void PeerSelector::selectAllParents() { - CachePeer *p; /* Add all alive parents */ - for (p = Config.peers; p; p = p->next) { + for (const auto &peer: CurrentCachePeers()) { + const auto p = peer.get(); /* XXX: neighbors.c lacks a public interface for enumerating * parents to a request so we have to dig some here.. */ @@ -887,7 +888,7 @@ PeerSelector::selectAllParents() * simply are not configured to handle the request. */ /* Add default parent as a last resort */ - if ((p = getDefaultParent(this))) { + if (const auto p = getDefaultParent(this)) { addSelection(p, DEFAULT_PARENT); } } diff --git a/src/peer_sourcehash.cc b/src/peer_sourcehash.cc index 1a2e1083ed..2cd54c02f4 100644 --- a/src/peer_sourcehash.cc +++ b/src/peer_sourcehash.cc @@ -10,6 +10,7 @@ #include "squid.h" #include "CachePeer.h" +#include "CachePeers.h" #include "HttpRequest.h" #include "mgr/Registration.h" #include "neighbors.h" @@ -42,8 +43,6 @@ peerSourceHashInit(void) int K; int k; double P_last, X_last, Xn; - CachePeer *p; - CachePeer **P; char *t; /* Clean up */ @@ -55,7 +54,7 @@ peerSourceHashInit(void) n_sourcehash_peers = 0; /* find out which peers we have */ - for (p = Config.peers; p; p = p->next) { + for (const auto &p: CurrentCachePeers()) { if (!p->options.sourcehash) continue; @@ -76,8 +75,11 @@ peerSourceHashInit(void) sourcehash_peers = (CachePeer **)xcalloc(n_sourcehash_peers, sizeof(*sourcehash_peers)); + auto P = sourcehash_peers; /* Build a list of the found peers and calculate hashes and load factors */ - for (P = sourcehash_peers, p = Config.peers; p; p = p->next) { + for (const auto &peer: CurrentCachePeers()) { + const auto p = peer.get(); + if (!p->options.sourcehash) continue; @@ -125,7 +127,7 @@ peerSourceHashInit(void) for (k = 1; k <= K; ++k) { double Kk1 = (double) (K - k + 1); - p = sourcehash_peers[k - 1]; + const auto p = sourcehash_peers[k - 1]; p->sourcehash.load_multiplier = (Kk1 * (p->sourcehash.load_factor - P_last)) / Xn; p->sourcehash.load_multiplier += pow(X_last, Kk1); p->sourcehash.load_multiplier = pow(p->sourcehash.load_multiplier, 1.0 / Kk1); @@ -195,7 +197,6 @@ peerSourceHashSelectParent(PeerSelector *ps) static void peerSourceHashCachemgr(StoreEntry * sentry) { - CachePeer *p; int sumfetches = 0; storeAppendPrintf(sentry, "%24s %10s %10s %10s %10s\n", "Hostname", @@ -204,10 +205,10 @@ peerSourceHashCachemgr(StoreEntry * sentry) "Factor", "Actual"); - for (p = Config.peers; p; p = p->next) + for (const auto &p: CurrentCachePeers()) sumfetches += p->stats.fetches; - for (p = Config.peers; p; p = p->next) { + for (const auto &p: CurrentCachePeers()) { storeAppendPrintf(sentry, "%24s %10x %10f %10f %10f\n", p->name, p->sourcehash.hash, p->sourcehash.load_multiplier, diff --git a/src/peer_userhash.cc b/src/peer_userhash.cc index 7637af2bed..28c2756616 100644 --- a/src/peer_userhash.cc +++ b/src/peer_userhash.cc @@ -14,6 +14,7 @@ #include "auth/UserRequest.h" #include "CachePeer.h" +#include "CachePeers.h" #include "globals.h" #include "HttpRequest.h" #include "mgr/Registration.h" @@ -47,8 +48,6 @@ peerUserHashInit(void) int K; int k; double P_last, X_last, Xn; - CachePeer *p; - CachePeer **P; char *t; /* Clean up */ @@ -62,7 +61,7 @@ peerUserHashInit(void) peerUserHashRegisterWithCacheManager(); - for (p = Config.peers; p; p = p->next) { + for (const auto &p: CurrentCachePeers()) { if (!p->options.userhash) continue; @@ -81,8 +80,11 @@ peerUserHashInit(void) userhash_peers = (CachePeer **)xcalloc(n_userhash_peers, sizeof(*userhash_peers)); + auto P = userhash_peers; /* Build a list of the found peers and calculate hashes and load factors */ - for (P = userhash_peers, p = Config.peers; p; p = p->next) { + for (const auto &peer: CurrentCachePeers()) { + const auto p = peer.get(); + if (!p->options.userhash) continue; @@ -130,7 +132,7 @@ peerUserHashInit(void) for (k = 1; k <= K; ++k) { double Kk1 = (double) (K - k + 1); - p = userhash_peers[k - 1]; + const auto p = userhash_peers[k - 1]; p->userhash.load_multiplier = (Kk1 * (p->userhash.load_factor - P_last)) / Xn; p->userhash.load_multiplier += pow(X_last, Kk1); p->userhash.load_multiplier = pow(p->userhash.load_multiplier, 1.0 / Kk1); @@ -203,7 +205,6 @@ peerUserHashSelectParent(PeerSelector *ps) static void peerUserHashCachemgr(StoreEntry * sentry) { - CachePeer *p; int sumfetches = 0; storeAppendPrintf(sentry, "%24s %10s %10s %10s %10s\n", "Hostname", @@ -212,10 +213,10 @@ peerUserHashCachemgr(StoreEntry * sentry) "Factor", "Actual"); - for (p = Config.peers; p; p = p->next) + for (const auto &p: CurrentCachePeers()) sumfetches += p->stats.fetches; - for (p = Config.peers; p; p = p->next) { + for (const auto &p: CurrentCachePeers()) { storeAppendPrintf(sentry, "%24s %10x %10f %10f %10f\n", p->name, p->userhash.hash, p->userhash.load_multiplier, diff --git a/src/snmp_agent.cc b/src/snmp_agent.cc index 119eab1444..fb11790d01 100644 --- a/src/snmp_agent.cc +++ b/src/snmp_agent.cc @@ -11,6 +11,7 @@ #include "squid.h" #include "cache_snmp.h" #include "CachePeer.h" +#include "CachePeers.h" #include "globals.h" #include "mem/Meter.h" #include "mem/Stats.h" @@ -190,14 +191,14 @@ snmp_meshPtblFn(variable_list * Var, snint * ErrP) Ip::Address laddr; char *cp = nullptr; CachePeer *p = nullptr; - int cnt = 0; debugs(49, 5, "snmp_meshPtblFn: peer " << Var->name[LEN_SQ_MESH + 3] << " requested!"); *ErrP = SNMP_ERR_NOERROR; u_int index = Var->name[LEN_SQ_MESH + 3] ; - for (p = Config.peers; p != nullptr; p = p->next, ++cnt) { - if (p->index == index) { - laddr = p->in_addr ; + for (const auto &peer: CurrentCachePeers()) { + if (peer->index == index) { + laddr = peer->in_addr ; + p = peer.get(); break; } } diff --git a/src/snmp_core.cc b/src/snmp_core.cc index 6538497990..2c13bdb6c3 100644 --- a/src/snmp_core.cc +++ b/src/snmp_core.cc @@ -13,6 +13,7 @@ #include "base/AsyncCallbacks.h" #include "base/CbcPointer.h" #include "CachePeer.h" +#include "CachePeers.h" #include "client_db.h" #include "comm.h" #include "comm/Connection.h" @@ -26,6 +27,7 @@ #include "snmp_core.h" #include "SnmpRequest.h" #include "SquidConfig.h" +#include "SquidMath.h" #include "tools.h" static void snmpPortOpened(Ipc::StartListeningAnswer&); @@ -724,9 +726,9 @@ static oid * peer_Inst(oid * name, snint * len, mib_tree_entry * current, oid_ParseFn ** Fn) { oid *instance = nullptr; - CachePeer *peers = Config.peers; + const auto peersAvailable = CurrentCachePeers().size(); - if (peers == nullptr) { + if (!peersAvailable) { debugs(49, 6, "snmp peer_Inst: No Peers."); current = current->parent->parent->parent->leaves[1]; while ((current) && (!current->parsefunction)) @@ -746,9 +748,9 @@ peer_Inst(oid * name, snint * len, mib_tree_entry * current, oid_ParseFn ** Fn) int no = name[current->len] ; int i; // Note: This works because the Config.peers keeps its index according to its position. - for ( i=0 ; peers && (i < no) ; peers = peers->next, ++i ) ; + for (i = 0; Less(i, peersAvailable) && Less(i, no); ++i); - if (peers) { + if (Less(i, peersAvailable)) { debugs(49, 6, "snmp peer_Inst: Encode peer #" << i); instance = (oid *)xmalloc(sizeof(*name) * (current->len + 1 )); memcpy(instance, name, (sizeof(*name) * current->len )); diff --git a/src/stat.cc b/src/stat.cc index 86b69869da..eb78565cdd 100644 --- a/src/stat.cc +++ b/src/stat.cc @@ -12,6 +12,7 @@ #include "AccessLogEntry.h" #include "CacheDigest.h" #include "CachePeer.h" +#include "CachePeers.h" #include "client_side.h" #include "client_side_request.h" #include "comm/Connection.h" @@ -1574,7 +1575,6 @@ statPeerSelect(StoreEntry * sentry) { #if USE_CACHE_DIGESTS StatCounters *f = &statCounter; - CachePeer *peer; const int tot_used = f->cd.times_used + f->icp.times_used; /* totals */ @@ -1583,7 +1583,7 @@ statPeerSelect(StoreEntry * sentry) /* per-peer */ storeAppendPrintf(sentry, "\nPer-peer statistics:\n"); - for (peer = getFirstPeer(); peer; peer = getNextPeer(peer)) { + for (const auto &peer: CurrentCachePeers()) { if (peer->digest) peerDigestStatsReport(peer->digest, sentry); else -- 2.39.2