ref: 69e9efa847c03924caef9db2a0a4db586ddcc2f9
parent: 6fbf81a98f8f747444cfe0e7a0591f6de511e510
author: cinap_lenrek <cinap_lenrek@felloff.net>
date: Sun Oct 15 17:20:30 EDT 2023
ndb/dns: avoid blocking 9p on refresh, use shorter 15second query timeout, cleanup When writing "refresh" control message to trigger garbage collection and there are other (query) processes active, the 9p loop would block until these processes complete. Instead, defer the garbage collection operation to these outstanding processes once any of them completes, keeping the 9p loop un-contended for longer. When theres no other processes, we can garbage collect ourselfs immediately (just as before). Only acquire the dnvars lock on Twrite, so we can handle other 9p requests while garbage collection is going on. Also reduce the 60 second timeout to 15 second for queries. Other dns servers usually use something between 10 and 30 seconds. Avoid excessive nsec() and time() calls by having timems() function that only uses nsec() and then updates global nowms (millisecons) and now (seconds) variable. After the end of readnet(), timems() needs to be called anyway to update the query statistics so we can use that as the new "now".
--- a/sys/src/cmd/ndb/dn.c
+++ b/sys/src/cmd/ndb/dn.c
@@ -29,8 +29,8 @@
Lock;
ulong names; /* names allocated */
ulong oldest; /* longest we'll leave a name around */
- int active;
- int mutex;
+ int active; /* number of active processes */
+ int mutex; /* 0 or pid of process doing garbage collection */
ushort id; /* same size as in packet */
} dnvars;
@@ -149,6 +149,15 @@
noted(NDFLT); /* die */
}
+uvlong
+timems(void)
+{+ uvlong ms = nsec()/1000000L;
+ now = ms/1000L;
+ nowms = ms;
+ return ms;
+}
+
void
dninit(void)
{@@ -164,6 +173,8 @@
dnvars.names = 0;
dnvars.id = truerand(); /* don't start with same id every time */
+ timems();
+
notify(ding);
}
@@ -665,11 +676,12 @@
if(traceactivity)
dnslog("get: %d active by pid %d from %p",dnvars.active, getpid(), getcallerpc(&req));
- lock(&dnvars);
+
/*
* can't block here if we're already holding one
* of the dnvars.active (recursive). will deadlock.
*/
+ lock(&dnvars);
while(!recursive && dnvars.mutex){unlock(&dnvars);
sleep(100); /* tune; was 200 */
@@ -676,8 +688,6 @@
lock(&dnvars);
}
rv = ++dnvars.active;
- now = time(nil);
- nowns = nsec();
req->id = ++dnvars.id;
req->aux = nil;
unlock(&dnvars);
@@ -687,9 +697,11 @@
void
putactivity(int recursive)
{+ int pid = getpid();
+
if(traceactivity)
- dnslog("put: %d active by pid %d",- dnvars.active, getpid());
+ dnslog("put: %d active by pid %d", dnvars.active, pid);+
lock(&dnvars);
dnvars.active--;
assert(dnvars.active >= 0); /* "dnvars.active %d", dnvars.active */
@@ -698,15 +710,16 @@
* clean out old entries and check for new db periodicly
* can't block here if being called to let go a "recursive" lock
* or we'll deadlock waiting for ourselves to give up the dnvars.active.
+ * also don't block if we are the 9p process (needrefresh == pid).
*/
- if (recursive || dnvars.mutex ||
- (needrefresh == 0 && dnvars.active > 0)){+ if(recursive || dnvars.mutex
+ || dnvars.active > 0 && (needrefresh == 0 || needrefresh == pid)){unlock(&dnvars);
return;
}
/* wait till we're alone */
- dnvars.mutex = 1;
+ dnvars.mutex = pid;
while(dnvars.active > 0){unlock(&dnvars);
sleep(100); /* tune; was 100 */
@@ -714,13 +727,14 @@
}
unlock(&dnvars);
- db2cache(needrefresh);
-
+ db2cache(needrefresh != 0);
dnageall(0);
/* let others back in */
+ lock(&dnvars);
needrefresh = 0;
dnvars.mutex = 0;
+ unlock(&dnvars);
}
int
--- a/sys/src/cmd/ndb/dnresolve.c
+++ b/sys/src/cmd/ndb/dnresolve.c
@@ -235,7 +235,7 @@
* thus we don't wait very long for them.
*/
static void
-notestats(vlong start, int tmout, int type)
+notestats(long ms, int tmout, int type)
{qlock(&stats);
if (tmout) {@@ -245,7 +245,7 @@
else if (type == Tcname)
stats.tmoutcname++;
} else {- long wait10ths = NS2MS(nsec() - start) / 100;
+ long wait10ths = ms / 100;
if (wait10ths <= 0)
stats.under10ths[0]++;
@@ -559,31 +559,31 @@
/* timed read of reply. sets srcip. ibuf must be 64K to handle tcp answers. */
static int
-readnet(Query *qp, int medium, uchar *ibuf, uvlong endms, uchar **replyp,
- uchar *srcip)
+readnet(Query *qp, int medium, uchar *ibuf, uvlong endms, uchar **replyp, uchar *srcip)
{int len, fd;
long ms;
- vlong startns = nsec();
+ uvlong startms;
uchar *reply;
uchar lenbuf[2];
- len = -1; /* pessimism */
*replyp = nil;
memset(srcip, 0, IPaddrlen);
- ms = endms - NS2MS(startns);
- if (ms <= 0)
+
+ startms = nowms;
+ ms = (long)(endms - startms);
+ if (ms < 1)
return -1; /* taking too long */
+ len = -1; /* pessimism */
reply = ibuf;
- alarm(ms);
if (medium == Udp)
if (qp->udpfd < 0)
dnslog("readnet: qp->udpfd closed"); else {+ alarm(ms);
len = read(qp->udpfd, ibuf, Udphdrsize+Maxudpin);
alarm(0);
- notestats(startns, len < 0, qp->type);
if (len >= IPaddrlen)
memmove(srcip, ibuf, IPaddrlen);
if (len >= Udphdrsize) {@@ -598,24 +598,28 @@
if (fd < 0)
dnslog("readnet: %s: tcp fd unset for dest %I",qp->dp->name, qp->tcpip);
- else if (readn(fd, lenbuf, 2) != 2) {- dnslog("readnet: short read of 2-byte tcp msg size from %I",- qp->tcpip);
- /* probably a time-out */
- notestats(startns, 1, qp->type);
- } else {- len = lenbuf[0]<<8 | lenbuf[1];
- if (readn(fd, ibuf, len) != len) {- dnslog("readnet: short read of tcp data from %I",- qp->tcpip);
- /* probably a time-out */
- notestats(startns, 1, qp->type);
- len = -1;
+ else {+ alarm(ms);
+ if (readn(fd, lenbuf, 2) != 2) {+ alarm(0);
+ dnslog("readnet: short read of 2-byte tcp msg size from %I", qp->tcpip);+ } else {+ len = lenbuf[0]<<8 | lenbuf[1];
+ if (readn(fd, ibuf, len) != len) {+ alarm(0);
+ dnslog("readnet: short read of tcp data from %I", qp->tcpip);+ len = -1;
+ } else {+ alarm(0);
+ }
}
}
memmove(srcip, qp->tcpip, IPaddrlen);
}
- alarm(0);
+
+ ms = (long)(timems() - startms);
+ notestats(ms, len < 0, qp->type);
+
*replyp = reply;
return len;
}
@@ -631,14 +635,14 @@
{int len;
char *err;
- char tbuf[32];
uchar *reply;
uchar srcip[IPaddrlen];
RR *rp;
- for (; timems() < endms &&
- (len = readnet(qp, medium, ibuf, endms, &reply, srcip)) >= 0;
- freeanswers(mp)){+ for (;; freeanswers(mp)) {+ len = readnet(qp, medium, ibuf, endms, &reply, srcip);
+ if (len < 0)
+ break;
/* convert into internal format */
memset(mp, 0, sizeof *mp);
err = convM2DNS(reply, len, mp, nil);
@@ -675,20 +679,6 @@
return 0;
}
}
- if (timems() >= endms) {- ; /* query expired */
- } else if (0) {- /* this happens routinely when a read times out */
- dnslog("readreply: %s type %s: ns %I read error or eof "- "(returned %d): %r", qp->dp->name, rrname(qp->type,
- tbuf, sizeof tbuf), srcip, len);
- if (medium == Udp)
- for (rp = qp->nsrp; rp != nil; rp = rp->next)
- if (rp->type == Tns)
- dnslog("readreply: %s: query sent to "- "ns %s", qp->dp->name,
- rp->host->name);
- }
memset(mp, 0, sizeof *mp);
return -1;
}
@@ -985,9 +975,6 @@
char buf[32];
Dest *p;
- if(timems() >= qp->req->aborttime)
- return -1;
-
/*
* if we send tcp query, we just take the dest ip address from
* the udp header placed there by tcpquery().
@@ -1272,15 +1259,10 @@
*/
static int
tcpquery(Query *qp, DNSmsg *mp, int depth, uchar *ibuf, uchar *obuf, int len,
- ulong waitms, int inns, ushort req)
+ uvlong endms, int inns, ushort req)
{int rv = 0;
- uvlong endms;
- endms = timems() + waitms;
- if(endms > qp->req->aborttime)
- endms = qp->req->aborttime;
-
if (0)
dnslog("%s: udp reply truncated; retrying query via tcp to %I",qp->dp->name, qp->tcpip);
@@ -1338,10 +1320,13 @@
for(ndest = 2; ndest < Maxdest; ndest += 2){qp->ndest = ndest;
qp->tcpset = 0;
+
+ endms = nowms;
+ if(endms >= qp->req->aborttime)
+ break;
if (xmitquery(qp, Udp, depth, obuf, inns, len) < 0)
break;
-
- endms = timems() + waitms;
+ endms += waitms;
if(endms > qp->req->aborttime)
endms = qp->req->aborttime;
@@ -1361,8 +1346,10 @@
} else {/* whoops, it was truncated! ask again via tcp */
freeanswers(&m);
+ if(nowms >= endms)
+ break;
rv = tcpquery(qp, &m, depth, ibuf, obuf, len,
- waitms, inns, req); /* answer in m */
+ endms, inns, req); /* answer in m */
if (rv < 0) {freeanswers(&m);
break; /* failed via tcp too */
@@ -1516,8 +1503,8 @@
Query q;
memset(&req, 0, sizeof req);
- req.isslave = 1;
req.aborttime = timems() + Maxreqtm;
+ req.isslave = 1;
req.from = "internal";
queryinit(&q, dnlookup(root, Cin, 1), Tns, &req);
nsrp = randomize(dblookup(root, Cin, Tns, 0, 0));
--- a/sys/src/cmd/ndb/dns.c
+++ b/sys/src/cmd/ndb/dns.c
@@ -64,7 +64,7 @@
int mfd[2];
int needrefresh;
ulong now;
-vlong nowns;
+uvlong nowms;
int sendnotifies;
char *trace;
int traceactivity;
@@ -186,8 +186,6 @@
(cfg.resolver? "resolver ": ""), mntpt);
opendatabase();
- now = time(nil); /* open time files before we fork */
- nowns = nsec();
dnsuser = estrdup(getuser());
snprint(servefile, sizeof servefile, "/srv/dns%s", ext);
@@ -421,10 +419,6 @@
if(debug)
dnslog("%F", &job->request);- getactivity(&req, 0);
- req.aborttime = timems() + Maxreqtm;
- req.from = "9p";
-
switch(job->request.type){default:
warning("unknown request type %d", job->request.type);@@ -454,9 +448,17 @@
rread(job, mf);
break;
case Twrite:
- /* &req is handed to dnresolve() */
+ getactivity(&req, 0);
+ req.aborttime = timems() + Maxreqtm;
+ req.from = "9p";
rwrite(job, mf, &req);
- break;
+ freejob(job);
+ if(req.isslave){+ putactivity(0);
+ _exits(0);
+ }
+ putactivity(0);
+ continue;
case Tclunk:
rclunk(job, mf);
break;
@@ -470,18 +472,7 @@
rwstat(job, mf);
break;
}
-
freejob(job);
-
- /*
- * slave processes die after replying
- */
- if(req.isslave){- putactivity(0);
- _exits(0);
- }
-
- putactivity(0);
}
}
@@ -702,7 +693,7 @@
else if(strcmp(job->request.data, "dump")==0)
dndump("/lib/ndb/dnsdump");else if(strcmp(job->request.data, "refresh")==0)
- needrefresh = 1;
+ needrefresh = getpid();
else if(strcmp(job->request.data, "stats")==0)
dnstats("/lib/ndb/dnsstats"); else if(strncmp(job->request.data, "target ", 7)==0){--- a/sys/src/cmd/ndb/dns.h
+++ b/sys/src/cmd/ndb/dns.h
@@ -1,8 +1,3 @@
-#define NS2MS(ns) ((ns) / 1000000L)
-#define S2MS(s) ((s) * 1000LL)
-
-#define timems() NS2MS(nsec())
-
typedef struct Ndbtuple Ndbtuple;
enum
@@ -152,7 +147,7 @@
Maxactive= 250,
/* tune; was 8*1000; that was too short */
- Maxreqtm= 60*1000, /* max. ms to process a request */
+ Maxreqtm= 15*1000, /* max. ms to process a request */
Notauthoritative = 0,
Authoritative,
@@ -424,10 +419,10 @@
extern char *logfile;
extern int maxage; /* age of oldest entry in cache (secs) */
extern char mntpt[];
-extern int needrefresh;
+extern int needrefresh; /* set to pid of the process requesting flush */
extern int norecursion;
extern ulong now; /* time base */
-extern vlong nowns;
+extern uvlong nowms;
extern Area *owned;
extern int sendnotifies;
extern ulong target;
@@ -491,6 +486,7 @@
int rrtype(char*);
void slave(Request*);
int subsume(char*, char*);
+uvlong timems(void);
int tsame(int, int);
void unique(RR*);
void warning(char*, ...);
--- a/sys/src/cmd/ndb/dnsdebug.c
+++ b/sys/src/cmd/ndb/dnsdebug.c
@@ -22,7 +22,7 @@
char mntpt[Maxpath];
int needrefresh;
ulong now;
-vlong nowns;
+uvlong nowms;
char *trace;
int traceactivity;
char *zonerefreshprogram;
@@ -76,8 +76,6 @@
usage();
}ARGEND
- now = time(nil);
- nowns = nsec();
dninit();
fmtinstall('P', prettyrrfmt);opendatabase();
@@ -258,7 +256,7 @@
}
memset(&req, 0, sizeof req);
req.isslave = 1;
- req.aborttime = NS2MS(nowns) + Maxreqtm;
+ req.aborttime = timems() + Maxreqtm;
*l = dnresolve(rp->host->name, Cin, Ta, &req, nil, 0, Recurse, 0, nil);
if(*l == nil)
*l = dnresolve(rp->host->name, Cin, Taaaa, &req,
@@ -345,7 +343,7 @@
memset(&req, 0, sizeof req);
getactivity(&req, 0);
req.isslave = 1;
- req.aborttime = NS2MS(nowns) + Maxreqtm;
+ req.aborttime = timems() + Maxreqtm;
rr = dnresolve(buf, Cin, type, &req, nil, 0, Recurse, rooted, nil);
if(rr){ print("----------------------------\n");--- a/sys/src/cmd/ndb/dnsgetip.c
+++ b/sys/src/cmd/ndb/dnsgetip.c
@@ -13,7 +13,7 @@
char mntpt[Maxpath];
int needrefresh = 0;
ulong now = 0;
-vlong nowns = 0;
+uvlong nowms = 0;
int traceactivity = 0;
char *zonerefreshprogram;
@@ -36,7 +36,7 @@
memset(&req, 0, sizeof req);
getactivity(&req, 0);
req.isslave = 1;
- req.aborttime = NS2MS(nowns) + Maxreqtm;
+ req.aborttime = timems() + Maxreqtm;
rr = dnresolve(name, Cin, type, &req, nil, 0, Recurse, 0, &status);
neg = rrremneg(&rr);
--- a/sys/src/cmd/ndb/dnstcp.c
+++ b/sys/src/cmd/ndb/dnstcp.c
@@ -19,7 +19,7 @@
char mntpt[Maxpath];
int needrefresh;
ulong now;
-vlong nowns;
+uvlong nowms;
int traceactivity;
char *zonerefreshprogram;
@@ -98,7 +98,6 @@
/* loop on requests */
for(;; putactivity(0)){- now = time(nil);
memset(&repmsg, 0, sizeof repmsg);
len = readmsg(0, buf, sizeof buf);
if(len <= 0)
@@ -105,7 +104,7 @@
break;
getactivity(&req, 0);
- req.aborttime = timems() + S2MS(15*Min);
+ req.aborttime = timems() + 15*Min*1000;
rcode = 0;
memset(&reqmsg, 0, sizeof reqmsg);
err = convM2DNS(buf, len, &reqmsg, &rcode);
--- a/sys/src/cmd/ndb/dnudpserver.c
+++ b/sys/src/cmd/ndb/dnudpserver.c
@@ -131,7 +131,7 @@
{volatile int fd, len, op, rcode, served;
char *volatile err;
- volatile char tname[32];
+ volatile char tname[32], ipstr[64];
volatile uchar buf[Udphdrsize + Maxudp + 1024];
volatile DNSmsg reqmsg, repmsg;
Inprogress *volatile p;
@@ -165,8 +165,6 @@
if(setjmp(req.mret))
putactivity(0);
req.isslave = 0;
- req.id = 0;
- req.aborttime = 0;
/* loop on requests */
for(;; putactivity(0)){@@ -188,16 +186,16 @@
uh = (Udphdr*)buf;
len -= Udphdrsize;
- // dnslog("read received UDP from %I to %I",- // ((Udphdr*)buf)->raddr, ((Udphdr*)buf)->laddr);
+ // dnslog("read received UDP from %I to %I", uh->raddr, uh->laddr);+ snprint(ipstr, sizeof(ipstr), "%I", uh->raddr);
getactivity(&req, 0);
req.aborttime = timems() + Maxreqtm;
- req.from = smprint("%I", buf);- rcode = 0;
+ req.from = ipstr;
served++;
stats.qrecvdudp++;
+ rcode = 0;
err = convM2DNS(&buf[Udphdrsize], len, &reqmsg, &rcode);
if(err){/* first bytes in buf are source IP addr */
@@ -215,8 +213,7 @@
}
op = reqmsg.flags & Omask;
if(op != Oquery && op != Onotify){- dnslog("server: op %d from %I", reqmsg.flags & Omask,- buf);
+ dnslog("server: op %d from %I", reqmsg.flags & Omask, buf);goto freereq;
}
@@ -267,8 +264,6 @@
p->inuse = 0;
freereq:
- free(req.from);
- req.from = nil;
freeanswers(&reqmsg);
if(req.isslave){putactivity(0);