This commit removes the functionality introduced in
c26dc0e6aaefedc55ed5c35a5744b5c01ba39ea1 (which is also modified by a
few later commits), as well as
05f3a0d1e0359f604cc6162708f3f381eabcd1d7. Instead we modify the
startWait check in rxi_CheckCall to apply to both "reading" and
"writing" to enforce "idle dead" timeouts.
Why do this? First, let's start out with the following:
If an Rx call gets permanently "stuck", what happens? What should
happen?
Here, "stuck" means that either the server or client hangs while
processing the call. The server or client is waiting for something to
complete before it issues the next rx_Read() or rx_Write() call. In
various situations over the years, this has happened because the
server or client is waiting for a lock, waiting for local disk I/O to
complete, or waiting for some other arbitrary event to occur.
Currently, what happens with such a "hanging" call is a little
complex, and has several different results in different situations.
The behavior of a call in this "stuck" situation is handled by the
"idle dead" timeout of an Rx call/connection. This timeout is enforced
in rxi_CheckCall, in two different conditionals (if an "idle dead"
timeout is configured):
if (call->startWait && ((call->startWait + idleDeadTime) < now) &&
(call->flags & RX_CALL_READER_WAIT)) {
if (call->state == RX_STATE_ACTIVE) {
cerror = RX_CALL_TIMEOUT;
goto mtuout;
}
}
and
if (call->lastSendData && ((call->lastSendData + idleDeadTime) < now)) {
if (call->state == RX_STATE_ACTIVE) {
cerror = conn->service ? conn->service->idleDeadErr : RX_CALL_IDLE;
idle_timeout = 1;
goto mtuout;
}
}
The first of these handles the case where we are waiting to rx_Read()
from a call for too long (the other side of the call needs to give us
more data). The second handles the case where we are waiting to
rx_Write() for too long (the other side of the call needs to read some
of the data we sent previously).
This second case was added by commit
c26dc0e6aaefedc55ed5c35a5744b5c01ba39ea1, but it has the general
problem that this check does not check if anyone is actually trying to
write to the call, and just tries to keep track of the last time we
wrote to the call. So, we may have written some data to the call
successfully, and then we went off to do something else. We can then
kill the call later for taking too long to write to, even though
nobody is trying to write to it. This results in a few problems:
(1) When the fileserver is writing to the client, it may need to wait
for various locks and it may need to wait for local disk I/O to
complete. If this takes too long for any reason, the fileserver
will kill the call (currently with VNOSERVICE), but the thread
for servicing the call will still keep running until whatever the
fileserver was waiting for finishes.
(2) lastSendData is set whenever we send any ACK besides an
RX_ACK_PING_RESPONSE (as of commit
658d2f47281306dfd46c5eddcecaeadc3e3e7fa9). If we are the server,
and we send any such ACK (in particular, RX_ACK_REQUESTED is
common), the "idle dead" timer starts. This means the server can
easily kill a call for idleness even if the server has never sent
the client anything, and even if the server is still actively
reading from the client.
(3) When a client tries to issue an RPC for the server, the "idle
dead" timeout effectively becomes a hard dead timeout, since we
will write the RPC arguments to the Rx stream, and then wait for
the server to respond with the output arguments. During this
time, our 'lastSendData' is the last time we sent our arguments
to the server, and so the call must finish before
'call->lastSendData + idleDeadTime' is in the past.
In addition to this "idle dead" processing, commit
05f3a0d1e0359f604cc6162708f3f381eabcd1d7 appears to attempt to provide
"idle dead"-like behavior by disabling Rx keepalives at certain points
(when we're waiting for disk I/O), controlled by the application
process (currently only the fileserver). The idea is that if
keepalives are disabled, the server will just appear unreachable to
the client, and so if disk I/O takes too long, the client will just
kill the call because it looks like the server is gone. However, this
also has some problems:
(A) Clients send their own keepalives, and the server will still
respond to them. So, the server will not appear to be
inaccessible anyway. But even if it did work:
(B) This approach only accounts for delays in disk I/O, and not
anywhere else (we could hang for a wide variety of reasons). It
also requires the fileserver to decide when it's okay for a call
to be killed due to "idle dead" and when it's not, which
currently seems to be decided somewhat arbitrarily.
(C) This doesn't really let the client dictate its own "idle dead"
timeout for idleness specifically; it just looks like the server
went away.
(D) The fileserver would appear to be unreachable in this situation,
but it's not actually unreachable. This can be confusing to
clients, since distinguishing between a server that is completely
down vs just taking too long is an important distinction.
(E) As noted in (1) above, the fileserver thread will still keep
waiting for whatever it has been waiting for, even though the
call has been killed and is thus useless.
So instead of all of this stuff, just modify the rxi_CheckCall "idle
dead" check to depend on the call->startWait parameter instead. This
parameter will be set whenever anyone is waiting for something to
proceed in the call, whether that is waiting to read data or write
data. This should make "idle dead" processing much simpler, as it is
reduced to effectively: if we've been waiting for longer than N
seconds, kill the call.
This involves ripping out much of the code related to lastSendData and
rx_KeepAlive*. This means removing the call->lastSendData field and
the rx_SetServerIdleDeadErr function, since those were only used for
the behavior in
c26dc0e6aaefedc55ed5c35a5744b5c01ba39ea1. This also
means removing rx_KeepAliveOn and rx_KeepAliveOff, since those were
only used for the behavior in
05f3a0d1e0359f604cc6162708f3f381eabcd1d7. This commit also removes the
only known use of the VNOSERVICE error code, so add some comments
saying what this code was used for (especially since it is quite
different from other V* error codes).
Note that the behavior in (1) could actually be desirable in some
situations. In environments that have clients without "idle dead"
functionality, and those clients cannot be upgraded or reconfigured,
this commit means those clients may hang forever if the server hangs
forever. Some sites may want the fileserver to be able to kill such
hanging calls, so the client will not hang (even if it doesn't free up
the fileserver thread). However, such behavior should really be a
special case for such sites, and not be the default behavior (or only
behavior) for all sites. The fileserver should just be concerned with
maintaining its own threads and availability, and clients should
manage their own timeouts and handle hanging servers.
Thanks to Markus Koeberl, who originally brought attention to some of
the problematic behavior here, and helped investigate what was going
on in the fileserver.
Change-Id: Ie0497d24f1bf4ad7d30ab59061f96c3298f47d17
Reviewed-on: http://gerrit.openafs.org/10773
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
* not created or not online */
#define VVOLEXISTS 104 /* Volume already exists */
#define VNOSERVICE 105 /* Volume is not in service (i.e. it's
- * is out of funds, is obsolete, or somesuch) */
+ * is out of funds, is obsolete, or somesuch). This
+ * error code is no longer used, but was previously
+ * used by the OpenAFS fileserver to kill "idle" calls,
+ * and OpenAFS clients may interpret it that way. */
#define VOFFLINE 106 /* Volume is off line, for the reason
* given in the offline message */
#define VONLINE 107 /* Volume is already on line */
rx_GetCallAbortCode @332
rx_SetCallAbortCode @333
rx_RecordCallStatistics @334
- rx_KeepAliveOn @335
- rx_KeepAliveOff @336
+; rx_KeepAliveOn @335
+; rx_KeepAliveOff @336
rx_HostOf @337
rx_PortOf @338
rx_Init
rx_InitHost
rx_InterruptCall
-rx_KeepAliveOff
-rx_KeepAliveOn
rx_KeyCreate
rx_MyMaxSendSize
rx_NewCall
rx_InterruptCall
rx_IsServerConn
rx_IsUsingPktCksum
-rx_KeepAliveOff
-rx_KeepAliveOn
rx_KeyCreate
rx_MyMaxSendSize
rx_NewCall
service->minProcs = 0;
service->maxProcs = 1;
service->idleDeadTime = 60;
- service->idleDeadErr = 0;
service->connDeadTime = rx_connDeadTime;
service->executeRequestProc = serviceProc;
service->checkReach = 0;
conn->nSpecific = 0;
conn->specific = NULL;
rx_SetConnDeadTime(conn, service->connDeadTime);
- conn->idleDeadTime = service->idleDeadTime;
- conn->idleDeadDetection = service->idleDeadErr ? 1 : 0;
+ rx_SetConnIdleDeadTime(conn, service->idleDeadTime);
for (i = 0; i < RX_MAXCALLS; i++) {
conn->twind[i] = rx_initSendWindow;
conn->rwind[i] = rx_initReceiveWindow;
call->rprev = 0;
call->lastAcked = 0;
call->localStatus = call->remoteStatus = 0;
- call->lastSendData = 0;
if (flags & RX_CALL_READER_WAIT) {
#ifdef RX_ENABLE_LOCKS
* processing), and for the connection (so that we can discover
* idle connections) */
conn->lastSendTime = call->lastSendTime = clock_Sec();
- /* Let a set of retransmits trigger an idle timeout */
- if (!xmit->resending)
- call->lastSendData = call->lastSendTime;
}
/* When sending packets we need to follow these rules:
(p->length <= (rx_AckDataSize(call->rwind) + 4 * sizeof(afs_int32))))
{
conn->lastSendTime = call->lastSendTime = clock_Sec();
- /* Don't count keepalive ping/acks here, so idleness can be tracked. */
- if ((p->header.type != RX_PACKET_TYPE_ACK) ||
- ((((struct rx_ackPacket *)rx_DataOf(p))->reason != RX_ACK_PING) &&
- (((struct rx_ackPacket *)rx_DataOf(p))->reason !=
- RX_ACK_PING_RESPONSE)))
- call->lastSendData = call->lastSendTime;
}
}
if (idleDeadTime) {
/* see if we have a non-activity timeout */
- if (call->startWait && ((call->startWait + idleDeadTime) < now) &&
- (call->flags & RX_CALL_READER_WAIT)) {
+ if (call->startWait && ((call->startWait + idleDeadTime) < now)) {
if (call->state == RX_STATE_ACTIVE) {
cerror = RX_CALL_TIMEOUT;
goto mtuout;
}
}
-
- if (call->lastSendData && ((call->lastSendData + idleDeadTime) < now)) {
- if (call->state == RX_STATE_ACTIVE) {
- cerror = conn->service ? conn->service->idleDeadErr : RX_CALL_IDLE;
- idle_timeout = 1;
- goto mtuout;
- }
- }
}
}
rxi_ScheduleKeepAliveEvent(call);
}
-void
-rx_KeepAliveOff(struct rx_call *call)
-{
- MUTEX_ENTER(&call->lock);
- rxi_CancelKeepAliveEvent(call);
- MUTEX_EXIT(&call->lock);
-}
-
-void
-rx_KeepAliveOn(struct rx_call *call)
-{
- MUTEX_ENTER(&call->lock);
- rxi_KeepAliveOn(call);
- MUTEX_EXIT(&call->lock);
-}
-
static void
rxi_GrowMTUOn(struct rx_call *call)
{
"rqc=%u,%u, tqc=%u,%u, iovqc=%u,%u, "
"lstatus=%u, rstatus=%u, error=%d, timeout=%u, "
"resendEvent=%d, keepAliveEvt=%d, delayedAckEvt=%d, delayedAbortEvt=%d, abortCode=%d, abortCount=%d, "
- "lastSendTime=%u, lastRecvTime=%u, lastSendData=%u"
+ "lastSendTime=%u, lastRecvTime=%u"
#ifdef RX_ENABLE_LOCKS
", refCount=%u"
#endif
(afs_uint32)c->rqc, (afs_uint32)rqc, (afs_uint32)c->tqc, (afs_uint32)tqc, (afs_uint32)c->iovqc, (afs_uint32)iovqc,
(afs_uint32)c->localStatus, (afs_uint32)c->remoteStatus, c->error, c->timeout,
c->resendEvent?1:0, c->keepAliveEvent?1:0, c->delayedAckEvent?1:0, c->delayedAbortEvent?1:0,
- c->abortCode, c->abortCount, c->lastSendTime, c->lastReceiveTime, c->lastSendData
+ c->abortCode, c->abortCount, c->lastSendTime, c->lastReceiveTime
#ifdef RX_ENABLE_LOCKS
, (afs_uint32)c->refCount
#endif
/* Define procedure to set service dead time */
#define rx_SetIdleDeadTime(service,time) ((service)->idleDeadTime = (time))
-/*
- * Define error to return in server connections when failing to answer.
- * (server only) For example, AFS viced sends VNOSERVICE.
- */
-#define rx_SetServerIdleDeadErr(service,err) ((service)->idleDeadErr = (err))
-
/* Define procedures for getting and setting before and after execute-request procs */
#define rx_SetAfterProc(service,proc) ((service)->afterProc = (proc))
#define rx_SetBeforeProc(service,proc) ((service)->beforeProc = (proc))
u_short connDeadTime; /* Seconds until a client of this service will be declared dead, if it is not responding */
u_short idleDeadTime; /* Time a server will wait for I/O to start up again */
u_char checkReach; /* Check for asymmetric clients? */
- afs_int32 idleDeadErr;
int nSpecific; /* number entries in specific data */
void **specific; /* pointer to connection specific data */
#ifdef RX_ENABLE_LOCKS
int abortCount; /* number of times last error was sent */
u_int lastSendTime; /* Last time a packet was sent on this call */
u_int lastReceiveTime; /* Last time a packet was received for this call */
- u_int lastSendData; /* Last time a nonping was sent on this call */
void (*arrivalProc) (struct rx_call * call, void * mh, int index); /* Procedure to call when reply is received */
void *arrivalProcHandle; /* Handle to pass to replyFunc */
int arrivalProcArg; /* Additional arg to pass to reply Proc */
extern void rxi_CallError(struct rx_call *call, afs_int32 error);
extern void rx_SetConnSecondsUntilNatPing(struct rx_connection *conn,
afs_int32 seconds);
-extern void rx_KeepAliveOn(struct rx_call *call);
-extern void rx_KeepAliveOff(struct rx_call *call);
extern int rxs_Release(struct rx_securityClass *aobj);
#ifndef KERNEL
extern void rx_PrintTheseStats(FILE * file, struct rx_statistics *s, int size,
* not created or not online */
#define VVOLEXISTS 104 /* Volume already exists */
#define VNOSERVICE 105 /* Volume is not in service (i.e. it's
- * is out of funds, is obsolete, or somesuch) */
+ * is out of funds, is obsolete, or somesuch). This
+ * error code is no longer used, but was previously
+ * used by the OpenAFS fileserver to kill "idle" calls,
+ * and OpenAFS clients may interpret it that way. */
#define VOFFLINE 106 /* Volume is off line, for the reason
* given in the offline message */
#define VONLINE 107 /* Volume is already on line */
Error errorCode = 0; /* return code to caller */
struct rx_connection *tcon = rx_ConnectionOf(acall);
- rx_KeepAliveOff(acall);
-
if ((errorCode = CheckVnodeWithCall(Fid, volptr, cbv, targetptr, locktype)))
goto gvpdone;
#endif /* ADMIN_IMPLICIT_LOOKUP */
}
gvpdone:
- if (errorCode)
- rx_KeepAliveOn(acall);
return errorCode;
} /*GetVolumePackage */
{
Error fileCode = 0; /* Error code returned by the volume package */
- rx_KeepAliveOff(acall);
if (parentwhentargetnotdir) {
VPutVnode(&fileCode, parentwhentargetnotdir);
assert_vnode_success_or_salvaging(fileCode);
if (volptr) {
VPutVolumeWithCall(volptr, cbv);
}
- rx_KeepAliveOn(acall);
if (*client) {
PutClient(client);
GetStatus(targetptr, OutStatus, rights, anyrights,
&tparentwhentargetnotdir);
- rx_KeepAliveOn(acall); /* I/O done */
-
/* if a r/w volume, promise a callback to the caller */
if (VolumeWriteable(volptr))
SetCallBackStruct(AddCallBack(client->z.host, Fid), CallBack);
&rights, &anyrights)))
goto Bad_FetchStatus;
- rx_KeepAliveOn(acall);
-
/* set volume synchronization information */
SetVolumeSync(Sync, volptr);
&rights, &anyrights)))
goto Bad_BulkStatus;
- rx_KeepAliveOn(acall);
-
/* set volume synchronization information, but only once per call */
if (i == 0)
SetVolumeSync(Sync, volptr);
continue;
}
- rx_KeepAliveOn(acall);
-
/* set volume synchronization information, but only once per call */
if (!VolSync_set) {
SetVolumeSync(Sync, volptr);
goto Bad_StoreData;
}
- rx_KeepAliveOn(acall);
-
/* set volume synchronization information */
SetVolumeSync(Sync, volptr);
*/
if (parentwhentargetnotdir != NULL) {
tparentwhentargetnotdir = *parentwhentargetnotdir;
- rx_KeepAliveOff(acall);
VPutVnode(&fileCode, parentwhentargetnotdir);
- rx_KeepAliveOn(acall);
assert_vnode_success_or_salvaging(fileCode);
parentwhentargetnotdir = NULL;
}
if (errorCode && (!targetptr->changed_newTime))
goto Bad_StoreData;
- rx_KeepAliveOff(acall);
/* Update the status of the target's vnode */
Update_TargetVnodeStatus(targetptr, TVS_SDATA, client, InStatus,
targetptr, volptr, 0, 0);
- rx_KeepAliveOn(acall);
/* Get the updated File's status back to the caller */
GetStatus(targetptr, OutStatus, rights, anyrights,
VVnodeWriteToRead(&errorCode, targetptr);
assert_vnode_success_or_salvaging(errorCode);
- rx_KeepAliveOn(acall);
-
/* break call backs on the directory */
BreakCallBack(client->z.host, Fid, 0);
(parentwhentargetnotdir ? parentwhentargetnotdir
: targetptr), volptr, 0, 0);
- rx_KeepAliveOn(acall);
-
/* convert the write lock to a read lock before breaking callbacks */
VVnodeWriteToRead(&errorCode, targetptr);
assert_vnode_success_or_salvaging(errorCode);
parentptr->disk.linkCount,
client->z.InSameNetwork);
- rx_KeepAliveOn(acall);
-
/* Return the updated parent dir's status back to caller */
GetStatus(parentptr, OutDirStatus, rights, anyrights, 0);
Update_TargetVnodeStatus(targetptr, TVS_CFILE, client, InStatus,
parentptr, volptr, 0, 0);
- rx_KeepAliveOn(acall);
-
/* set up the return status for the parent dir and the newly created file, and since the newly created file is owned by the creator, give it PRSFS_ADMINISTER to tell the client its the owner of the file */
GetStatus(targetptr, OutFidStatus, rights | PRSFS_ADMINISTER, anyrights, parentptr);
GetStatus(parentptr, OutDirStatus, rights, anyrights, 0);
assert_vnode_success_or_salvaging(errorCode);
}
- rx_KeepAliveOn(acall);
-
/* break call back on NewDirFid, OldDirFid, NewDirFid and newFileFid */
BreakCallBack(client->z.host, NewDirFid, 0);
if (oldvptr != newvptr) {
Bad_Rename:
if (newfileptr) {
- rx_KeepAliveOff(acall);
VPutVnode(&fileCode, newfileptr);
assert_vnode_success_or_salvaging(fileCode);
}
VVnodeWriteToRead(&errorCode, parentptr);
assert_vnode_success_or_salvaging(errorCode);
- rx_KeepAliveOn(acall);
-
/* break call back on the parent dir */
BreakCallBack(client->z.host, DirFid, 0);
VVnodeWriteToRead(&errorCode, parentptr);
assert_vnode_success_or_salvaging(errorCode);
- rx_KeepAliveOn(acall);
-
/* break call back on DirFid */
BreakCallBack(client->z.host, DirFid, 0);
/*
VVnodeWriteToRead(&errorCode, parentptr);
assert_vnode_success_or_salvaging(errorCode);
- rx_KeepAliveOn(acall);
-
/* break call back on DirFid */
BreakCallBack(client->z.host, DirFid, 0);
VVnodeWriteToRead(&errorCode, parentptr);
assert_vnode_success_or_salvaging(errorCode);
- rx_KeepAliveOn(acall);
-
/* break call back on DirFid and fileFid */
BreakCallBack(client->z.host, DirFid, 0);
/* if no more locks left, a callback would be triggered here */
if (targetptr->disk.lock.lockCount <= 0) {
- rx_KeepAliveOn(acall);
/* convert the write lock to a read lock before breaking callbacks */
VVnodeWriteToRead(&errorCode, targetptr);
assert_vnode_success_or_salvaging(errorCode);
inet_ntoa(logHostAddr), ntohs(rxr_PortOf(rx_ConnectionOf(Call)))));
return ENOENT; /* is this proper error code? */
} else {
- rx_KeepAliveOff(Call);
/*
* See if the file has several links (from other volumes). If it
* does, then we have to make a copy before changing it to avoid
rx_SetMinProcs(tservice, 3);
rx_SetMaxProcs(tservice, lwps);
rx_SetCheckReach(tservice, 1);
- rx_SetServerIdleDeadErr(tservice, VNOSERVICE);
tservice =
rx_NewService(0, RX_STATS_SERVICE_ID, "rpcstats", securityClasses,
* not created or not online */
#define VVOLEXISTS 104 /* Volume already exists */
#define VNOSERVICE 105 /* Volume is not in service (i.e. it's
- * is out of funds, is obsolete, or somesuch) */
+ * is out of funds, is obsolete, or somesuch). This
+ * error code is no longer used, but was previously
+ * used by the OpenAFS fileserver to kill "idle" calls,
+ * and OpenAFS clients may interpret it that way. */
#define VOFFLINE 106 /* Volume is off line, for the reason
* given in the offline message */
#define VONLINE 107 /* Volume is already on line */
git://git.openafs.org / openafs.git / commitdiff