viced: consistently enforce host thread quota for ICBS(3) 73/13873/3
authorMark Vitale <mvitale@sinenomine.net>
Tue, 17 Sep 2019 19:14:44 +0000 (15:14 -0400)
committerBenjamin Kaduk <kaduk@mit.edu>
Fri, 27 Sep 2019 15:10:28 +0000 (11:10 -0400)
From time to time, the fileserver may issue potentially long-running
RXAFSCB_* RPCs back to a host (client).  If these are holding h_Lock_r
(host->lock) while running, they may cause other service threads for the
same host (client) to block.

In order to prevent a given host from tying up too many service threads
in this way, the fileserver enforces a quota limiting how many threads
can be waiting for h_Lock_r on a particular host while waiting for one
of the following RPCs to complete:
- RXAFSCB_TellMeABoutYourself (TMAY)
- RXAFSCB_WhoAreYou
- RXAFSCB_ProbeUuid
- RXAFSCB_InitCallBackState (ICBS)
- RXAFSCB_InitCallBackState3 (ICBS3)

Note: Although some of these RPCs are relatively lightweight, they may
still experience network delays.

This quota is enforced by calling h_threadquota() in h_Lookup_r and
h_GetHost_r.  The quota check is enabled for a given host by turning on
host->hostFlags HWHO_INPROGRESS for the duration of the RXAFSCB_* RPC.
The quota check is only needed, and should only be enabled, when the RPC
is issued while h_Lock_r is held.

However, there are a few paths to ICBS(3) where h_Lock_r is held but
HWHO_INPROGRESS is not set.  A delay in those paths may allow a host to
consume an unlimited number of fileserver threads.  One such path
observed in a field report was SRXAFS_FetchStatus -> CallPreamble ->
BreakDelayedCallBacks_r -> RXAFSCB_ICBS3.

Instead, enable host thread quotas for all remaining unregulated ICBS(3)
RPCs.

Change-Id: I70b96055ff80d8650bdbaec0302b7d18a8f22d56
Reviewed-on: https://gerrit.openafs.org/13873
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

src/viced/callback.c

index f685398..980d7e5 100644 (file)
@@ -1035,6 +1035,7 @@ BreakDelayedCallBacks_r(struct host *host)
     cbstuff.nbreakers++;
     if (!(host->z.hostFlags & RESETDONE) && !(host->z.hostFlags & HOSTDELETED)) {
        host->z.hostFlags &= ~ALTADDR;  /* alternate addresses are invalid */
+       host->z.hostFlags |= HWHO_INPROGRESS; /* ICBS(3) invokes thread quota */
        cb_conn = host->z.callback_rxcon;
        rx_GetConnection(cb_conn);
        if (host->z.interface) {
@@ -1049,6 +1050,7 @@ BreakDelayedCallBacks_r(struct host *host)
        cb_conn = NULL;
        H_LOCK;
        host->z.hostFlags |= ALTADDR;   /* alternate addresses are valid */
+       host->z.hostFlags &= ~HWHO_INPROGRESS;
        if (code) {
            if (ShowProblems) {
                ViceLog(0,
@@ -1642,6 +1644,7 @@ ClearHostCallbacks_r(struct host *hp, int locked)
     } else if (!(hp->z.hostFlags & HOSTDELETED)) {
        /* host is up, try a call */
        hp->z.hostFlags &= ~ALTADDR;    /* alternate addresses are invalid */
+       hp->z.hostFlags |= HWHO_INPROGRESS;   /* enable host thread quota enforcement */
        cb_conn = hp->z.callback_rxcon;
        rx_GetConnection(hp->z.callback_rxcon);
        if (hp->z.interface) {
@@ -1656,6 +1659,7 @@ ClearHostCallbacks_r(struct host *hp, int locked)
        cb_conn = NULL;
        H_LOCK;
        hp->z.hostFlags |= ALTADDR;     /* alternate addresses are valid */
+       hp->z.hostFlags &= ~HWHO_INPROGRESS;
        if (code) {
            /* failed, mark host down and need reset */
            hp->z.hostFlags |= VENUSDOWN;