rx: Remove delays in multi_End_Ignore 95/14595/3
authorAndrew Deason <adeason@sinenomine.net>
Fri, 16 Apr 2021 16:11:35 +0000 (11:11 -0500)
committerBenjamin Kaduk <kaduk@mit.edu>
Thu, 27 May 2021 05:40:10 +0000 (01:40 -0400)
When using our multi_Rx mechanism, callers can use either multi_End or
multi_End_Ignore at the end of the multi_Rx block. Among other things,
these macros run 'rx_EndCall(call, code)' for each call in the
multi_Rx invocation that hasn't already ended. For multi_End, 'code'
is RX_USER_ABORT, and for multi_End_Ignore, 'code' is 0; the macros
are otherwise equivalent.

When multi_End is used, this means any un-ended calls are aborted with
RX_USER_ABORT, and the call immediately ends. But when
multi_End_Ignore is used, the call is not aborted, and so we must wait
for the peer to acknowledge that it has received our packets before
ending (done via an rxi_ReadProc call in rx_EndCall).

This means that if a caller multi_Abort's out of a multi call and uses
multi_End_Ignore, we'll wait for the peer to acknowledge our packets
for all of the calls we haven't processed. Waiting for that is a
complete waste of time, since we don't care about the results of those
calls (since we multi_Abort'd). This doesn't matter much if those
calls are responded to promptly, but if the peer is not up or is just
slow, it can cause us to wait several seconds until we timeout.

There are currently only three users of multi_End_Ignore:

- DoProbe in src/ubik/recovery.c

- MultiBreakCallBackAlternateAddress_r in src/viced/callback.c

- MultiProbeAlternateAddress_r in src/viced/callback.c

All of these use multi_Rx to try and probe multiple IPs for the same
machine in parallel, and so some of the calls may very well be trying
to contact unreachable IPs; we only need one to work for the call to
succeed.

To avoid the unnecessary delays in these codepaths, convert them to
use multi_End. Change multi_End_Ignore to be the same as multi_End,
and multi_Finalize_Ignore to the same as multi_Finalize, to make sure
the bad behavior is not used. The _Ignore macros/functions are now
unused in the tree, but keep them around for now since
multi_Finalize_Ignore is exported by libafsrpc.

Thanks to mbarbosa@sinenomine.net for discovering this weird behavior.

Change-Id: I65536a0975bd7a16bb805555943c032c5e6afdf3
Reviewed-on: https://gerrit.openafs.org/14595
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

src/rx/rx_multi.c
src/rx/rx_multi.h
src/ubik/recovery.c
src/viced/callback.c

index 85cca41..7c79767 100644 (file)
@@ -121,20 +121,9 @@ multi_Finalize(struct multi_handle *mh)
     osi_Free(mh, sizeof(struct multi_handle));
 }
 
-/* ignores all remaining multiRx calls */
+/* Deprecated; use multi_Finalize() instead. */
 void
 multi_Finalize_Ignore(struct multi_handle *mh)
 {
-    int i;
-    int nCalls = mh->nConns;
-    for (i = 0; i < nCalls; i++) {
-       struct rx_call *call = mh->calls[i];
-       if (call)
-           rx_EndCall(call, 0);
-    }
-    MUTEX_DESTROY(&mh->lock);
-    CV_DESTROY(&mh->cv);
-    osi_Free(mh->calls, sizeof(struct rx_call *) * nCalls);
-    osi_Free(mh->ready, sizeof(short) * nCalls);
-    osi_Free(mh, sizeof(struct multi_handle));
+    multi_Finalize(mh);
 }
index de67ae3..80bf1ec 100644 (file)
@@ -54,9 +54,7 @@ struct multi_handle {
        multi_Finalize(multi_h);\
     } while (0)
 
-/* Ignore remaining multi RPC's */
-#define multi_End_Ignore\
-       multi_Finalize_Ignore(multi_h);\
-    } while (0)
+/* Deprecated; use multi_End instead. */
+#define multi_End_Ignore multi_End
 
 #endif /* _RX_MULTI_     End of rx_multi.h */
index a53aa59..e622505 100644 (file)
@@ -946,7 +946,7 @@ DoProbe(struct ubik_server *server)
 
            multi_Abort;
        }
-    } multi_End_Ignore;
+    } multi_End;
 
     if (success_i >= 0) {
        UBIK_ADDR_LOCK;
index 0e38a95..97cd0a1 100644 (file)
@@ -3074,7 +3074,7 @@ MultiBreakCallBackAlternateAddress_r(struct host *host,
            multi_Abort;
        }
     }
-    multi_End_Ignore;
+    multi_End;
     H_LOCK;
     /* Destroy all connections except the one on which we succeeded */
     for (i = 0; i < j; i++)
@@ -3200,7 +3200,7 @@ MultiProbeAlternateAddress_r(struct host *host)
        FS_STATE_UNLOCK;
 #endif
     }
-    multi_End_Ignore;
+    multi_End;
     H_LOCK;
     /* Destroy all connections except the one on which we succeeded */
     for (i = 0; i < j; i++)