Skip to content

Commit 16cafa1

Browse files
eduard-bagdasaryansquid-anubis
authored andcommitted
Report cache_peer context in probe and standby pool messages (#1960)
The absence of the usual "current master transaction:..." detail in certain errors raises "Has Squid lost the current transaction context?" red flags: ERROR: Connection to peerXyz failed In some cases, Squid may have lost that context, but for cache_peer TCP probes, Squid has not because those probes are not associated with master transactions. It is difficult to distinguish the two cases because no processing context is reported. To address those concerns, we now report current cache_peer probing context (instead of just not reporting absent master transaction context): ERROR: Connection to peerXyz failed current cache_peer probe: peerXyzIP When maintaining a cache_peer standy=N connection pool, Squid has and now reports both contexts, attributing messages to pool maintenance: ERROR: Connection to peerXyz failed current cache_peer standby pool: peerXyz current master transaction: master1234 The new PrecomputedCodeContext class handles both reporting cases and can be reused whenever the cost of pre-computing detailCodeContext() output is acceptable.
1 parent 75c97d2 commit 16cafa1

File tree

8 files changed

+83
-14
lines changed

8 files changed

+83
-14
lines changed

src/CachePeer.cc

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,15 @@
88

99
#include "squid.h"
1010
#include "acl/Gadgets.h"
11+
#include "base/PrecomputedCodeContext.h"
1112
#include "CachePeer.h"
1213
#include "defines.h"
1314
#include "neighbors.h"
1415
#include "NeighborTypeDomainList.h"
1516
#include "pconn.h"
1617
#include "PeerDigest.h"
1718
#include "PeerPoolMgr.h"
19+
#include "sbuf/Stream.h"
1820
#include "SquidConfig.h"
1921
#include "util.h"
2022

@@ -23,7 +25,8 @@ CBDATA_CLASS_INIT(CachePeer);
2325
CachePeer::CachePeer(const char * const hostname):
2426
name(xstrdup(hostname)),
2527
host(xstrdup(hostname)),
26-
tlsContext(secure, sslContext)
28+
tlsContext(secure, sslContext),
29+
probeCodeContext(new PrecomputedCodeContext("cache_peer probe", ToSBuf("current cache_peer probe: ", *this)))
2730
{
2831
Tolower(host); // but .name preserves original spelling
2932
}

src/CachePeer.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
#include "acl/forward.h"
1313
#include "base/CbcPointer.h"
14+
#include "base/forward.h"
1415
#include "enums.h"
1516
#include "http/StatusCode.h"
1617
#include "icp_opcode.h"
@@ -224,6 +225,8 @@ class CachePeer
224225
int front_end_https = 0; ///< 0 - off, 1 - on, 2 - auto
225226
int connection_auth = 2; ///< 0 - off, 1 - on, 2 - auto
226227

228+
PrecomputedCodeContextPointer probeCodeContext;
229+
227230
private:
228231
void countFailure();
229232
};

src/PeerPoolMgr.cc

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include "squid.h"
1010
#include "AccessLogEntry.h"
1111
#include "base/AsyncCallbacks.h"
12+
#include "base/PrecomputedCodeContext.h"
1213
#include "base/RunnersRegistry.h"
1314
#include "CachePeer.h"
1415
#include "CachePeers.h"
@@ -23,18 +24,27 @@
2324
#include "neighbors.h"
2425
#include "pconn.h"
2526
#include "PeerPoolMgr.h"
27+
#include "sbuf/Stream.h"
2628
#include "security/BlindPeerConnector.h"
2729
#include "SquidConfig.h"
2830

2931
CBDATA_CLASS_INIT(PeerPoolMgr);
3032

3133
PeerPoolMgr::PeerPoolMgr(CachePeer *aPeer): AsyncJob("PeerPoolMgr"),
3234
peer(cbdataReference(aPeer)),
33-
request(),
3435
transportWait(),
3536
encryptionWait(),
3637
addrUsed(0)
3738
{
39+
const auto mx = MasterXaction::MakePortless<XactionInitiator::initPeerPool>();
40+
41+
codeContext = new PrecomputedCodeContext("cache_peer standby pool", ToSBuf("current cache_peer standby pool: ", *peer,
42+
Debug::Extra, "current master transaction: ", mx->id));
43+
44+
// ErrorState, getOutgoingAddress(), and other APIs may require a request.
45+
// We fake one. TODO: Optionally send this request to peers?
46+
request = new HttpRequest(Http::METHOD_OPTIONS, AnyP::PROTO_HTTP, "http", "*", mx);
47+
request->url.host(peer->host);
3848
}
3949

4050
PeerPoolMgr::~PeerPoolMgr()
@@ -46,13 +56,6 @@ void
4656
PeerPoolMgr::start()
4757
{
4858
AsyncJob::start();
49-
50-
const auto mx = MasterXaction::MakePortless<XactionInitiator::initPeerPool>();
51-
// ErrorState, getOutgoingAddress(), and other APIs may require a request.
52-
// We fake one. TODO: Optionally send this request to peers?
53-
request = new HttpRequest(Http::METHOD_OPTIONS, AnyP::PROTO_HTTP, "http", "*", mx);
54-
request->url.host(peer->host);
55-
5659
checkpoint("peer initialized");
5760
}
5861

@@ -228,7 +231,14 @@ PeerPoolMgr::checkpoint(const char *reason)
228231
void
229232
PeerPoolMgr::Checkpoint(const Pointer &mgr, const char *reason)
230233
{
231-
CallJobHere1(48, 5, mgr, PeerPoolMgr, checkpoint, reason);
234+
if (!mgr.valid()) {
235+
debugs(48, 5, reason << " but no mgr");
236+
return;
237+
}
238+
239+
CallService(mgr->codeContext, [&] {
240+
CallJobHere1(48, 5, mgr, PeerPoolMgr, checkpoint, reason);
241+
});
232242
}
233243

234244
/// launches PeerPoolMgrs for peers configured with standby.limit
@@ -254,7 +264,9 @@ PeerPoolMgrsRr::syncConfig()
254264
if (p->standby.limit) {
255265
p->standby.mgr = new PeerPoolMgr(p);
256266
p->standby.pool = new PconnPool(p->name, p->standby.mgr);
257-
AsyncJob::Start(p->standby.mgr.get());
267+
CallService(p->standby.mgr->codeContext, [&] {
268+
AsyncJob::Start(p->standby.mgr.get());
269+
});
258270
}
259271
}
260272
}

src/PeerPoolMgr.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#define SQUID_SRC_PEERPOOLMGR_H
1111

1212
#include "base/AsyncJob.h"
13+
#include "base/forward.h"
1314
#include "base/JobWait.h"
1415
#include "comm/forward.h"
1516
#include "security/forward.h"
@@ -32,6 +33,8 @@ class PeerPoolMgr: public AsyncJob
3233
explicit PeerPoolMgr(CachePeer *aPeer);
3334
~PeerPoolMgr() override;
3435

36+
PrecomputedCodeContextPointer codeContext;
37+
3538
protected:
3639
/* AsyncJob API */
3740
void start() override;

src/base/Makefile.am

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ libbase_la_SOURCES = \
5151
OnOff.h \
5252
Packable.h \
5353
PackableStream.h \
54+
PrecomputedCodeContext.h \
5455
Random.cc \
5556
Random.h \
5657
RandomUuid.cc \

src/base/PrecomputedCodeContext.h

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
/*
2+
* Copyright (C) 1996-2024 The Squid Software Foundation and contributors
3+
*
4+
* Squid software is distributed under GPLv2+ license and includes
5+
* contributions from numerous individuals and organizations.
6+
* Please see the COPYING and CONTRIBUTORS files for details.
7+
*/
8+
9+
#ifndef SQUID_SRC_BASE_PRECOMPUTEDCODECONTEXT_H
10+
#define SQUID_SRC_BASE_PRECOMPUTEDCODECONTEXT_H
11+
12+
#include "base/CodeContext.h"
13+
#include "base/InstanceId.h"
14+
#include "sbuf/SBuf.h"
15+
16+
#include <ostream>
17+
18+
/// CodeContext with constant details known at construction time
19+
class PrecomputedCodeContext: public CodeContext
20+
{
21+
public:
22+
typedef RefCount<PrecomputedCodeContext> Pointer;
23+
24+
PrecomputedCodeContext(const char *gist, const SBuf &detail): gist_(gist), detail_(detail)
25+
{}
26+
27+
/* CodeContext API */
28+
ScopedId codeContextGist() const override { return ScopedId(gist_); }
29+
std::ostream &detailCodeContext(std::ostream &os) const override { return os << Debug::Extra << detail_; }
30+
31+
private:
32+
const char *gist_; ///< the id used in codeContextGist()
33+
const SBuf detail_; ///< the detail used in detailCodeContext()
34+
};
35+
36+
#endif /* SQUID_SRC_BASE_PRECOMPUTEDCODECONTEXT_H */
37+

src/base/forward.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ class AsyncJob;
1515
class CallDialer;
1616
class CodeContext;
1717
class DelayedAsyncCalls;
18+
class PrecomputedCodeContext;
1819
class Raw;
1920
class RegexPattern;
2021
class ScopedId;
@@ -28,6 +29,7 @@ template<class Answer> class AsyncCallback;
2829
typedef CbcPointer<AsyncJob> AsyncJobPointer;
2930
typedef RefCount<CodeContext> CodeContextPointer;
3031
using AsyncCallPointer = RefCount<AsyncCall>;
32+
using PrecomputedCodeContextPointer = RefCount<PrecomputedCodeContext>;
3133

3234
#endif /* SQUID_SRC_BASE_FORWARD_H */
3335

src/neighbors.cc

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include "base/EnumIterator.h"
1515
#include "base/IoManip.h"
1616
#include "base/PackableStream.h"
17+
#include "base/PrecomputedCodeContext.h"
1718
#include "CacheDigest.h"
1819
#include "CachePeer.h"
1920
#include "CachePeers.h"
@@ -569,9 +570,12 @@ neighborsUdpPing(HttpRequest * request,
569570

570571
reqnum = icpSetCacheKey((const cache_key *)entry->key);
571572

573+
const auto savedContext = CodeContext::Current();
572574
for (size_t i = 0; i < Config.peers->size(); ++i) {
573575
const auto p = &Config.peers->nextPeerToPing(i);
574576

577+
CodeContext::Reset(p->probeCodeContext);
578+
575579
debugs(15, 5, "candidate: " << *p);
576580

577581
if (!peerWouldBePinged(p, ps))
@@ -660,6 +664,7 @@ neighborsUdpPing(HttpRequest * request,
660664
if ((p->type != PEER_MULTICAST) && (p->stats.probe_start == 0))
661665
p->stats.probe_start = squid_curtime;
662666
}
667+
CodeContext::Reset(savedContext);
663668

664669
/*
665670
* How many replies to expect?
@@ -1053,8 +1058,7 @@ int
10531058
neighborUp(const CachePeer * p)
10541059
{
10551060
if (!p->tcp_up) {
1056-
// TODO: When CachePeer gets its own CodeContext, pass that context instead of nullptr
1057-
CallService(nullptr, [&] {
1061+
CallService(p->probeCodeContext, [&] {
10581062
peerProbeConnect(const_cast<CachePeer*>(p));
10591063
});
10601064
return 0;
@@ -1170,8 +1174,12 @@ peerDnsRefreshCheck(void *)
11701174
static void
11711175
peerDnsRefreshStart()
11721176
{
1173-
for (const auto &p: CurrentCachePeers())
1177+
const auto savedContext = CodeContext::Current();
1178+
for (const auto &p: CurrentCachePeers()) {
1179+
CodeContext::Reset(p->probeCodeContext);
11741180
ipcache_nbgethostbyname(p->host, peerDNSConfigure, p.get());
1181+
}
1182+
CodeContext::Reset(savedContext);
11751183

11761184
peerScheduleDnsRefreshCheck(3600.0);
11771185
}

0 commit comments

Comments
 (0)