winbind client: avoid vicious cycle created by client retry

This patch cancels the retry policy of the winbind client.

When winbindd fails to respond to a request within 30 seconds,
the winbind client closes the connection and retries up to 10
times.

In some cases, delayed response is a result of multiple
requests from multiple clients piling up on the winbind domain
child process. Retrying just piles more and more requests,
creating a vicious cycle.

Even in the case of a single request taking long to complete,
there's no point in retrying because the retry request would just
wait for the current request to complete. Better to wait patiently.

There's one possible benefit in the retry, namely that winbindd typically
caches the results, and therefore a retry might take a cached result, so
the net effect of the retry may be to increase the timeout to 300 seconds.
But a more straightforward way to have a 300 second timeout is to modify the
timeout. Therefore the timeout is modified from 30 seconds to 300 seconds

(IMHO 300 seconds is too much, but we have "winbind rquest timeout"
with a default of 60 to make sure the request completes or fails
within 60 seconds)

BUG: https://bugzilla.samba.org/show_bug.cgi?id=11397

Signed-off-by: Uri Simchoni <urisimchoni@gmail.com>
Reviewed-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Volker Lendecke <vl@samba.org>

diff --git a/nsswitch/wb_common.c b/nsswitch/wb_common.c
index 3194df197b74042f42af80f610e9645eee76ca06..139f0106669cc499d7d09cc6b3ef1c24256b5d02 100644 (file)
--- a/nsswitch/wb_common.c
+++ b/nsswitch/wb_common.c
@@ -535,7 +535,7 @@ static int winbind_read_sock(struct winbindd_context *ctx,
 
                if (ret == 0) {
                        /* Not ready for read yet... */
-                       if (total_time >= 30) {
+                       if (total_time >= 300) {
                                /* Timeout */
                                winbind_close_sock(ctx);
                                return -1;
@@ -719,20 +719,16 @@ NSS_STATUS winbindd_request_response(struct winbindd_context *ctx,
                                     struct winbindd_response *response)
 {
        NSS_STATUS status = NSS_STATUS_UNAVAIL;
-       int count = 0;
        struct winbindd_context *wb_ctx = ctx;
 
        if (ctx == NULL) {
                wb_ctx = &wb_global_ctx;
        }
 
-       while ((status == NSS_STATUS_UNAVAIL) && (count < 10)) {
-               status = winbindd_send_request(wb_ctx, req_type, 0, request);
-               if (status != NSS_STATUS_SUCCESS)
-                       return(status);
-               status = winbindd_get_response(wb_ctx, response);
-               count += 1;
-       }
+       status = winbindd_send_request(wb_ctx, req_type, 0, request);
+       if (status != NSS_STATUS_SUCCESS)
+               return (status);
+       status = winbindd_get_response(wb_ctx, response);
 
        return status;
 }
@@ -743,20 +739,16 @@ NSS_STATUS winbindd_priv_request_response(struct winbindd_context *ctx,
                                          struct winbindd_response *response)
 {
        NSS_STATUS status = NSS_STATUS_UNAVAIL;
-       int count = 0;
        struct winbindd_context *wb_ctx = ctx;
 
        if (ctx == NULL) {
                wb_ctx = &wb_global_ctx;
        }
 
-       while ((status == NSS_STATUS_UNAVAIL) && (count < 10)) {
-               status = winbindd_send_request(wb_ctx, req_type, 1, request);
-               if (status != NSS_STATUS_SUCCESS)
-                       return(status);
-               status = winbindd_get_response(wb_ctx, response);
-               count += 1;
-       }
+       status = winbindd_send_request(wb_ctx, req_type, 1, request);
+       if (status != NSS_STATUS_SUCCESS)
+               return (status);
+       status = winbindd_get_response(wb_ctx, response);
 
        return status;
 }