Clean up backend_retry and its callers.

diff --git a/servers/lloadd/backend.c b/servers/lloadd/backend.c
index 42e903f3f06568fe322f282e72a4b2b211ba97a7..872cafe1dad851a9032a924683ca9c5c8e4390b3 100644 (file)
--- a/servers/lloadd/backend.c
+++ b/servers/lloadd/backend.c
@@ -50,13 +50,14 @@ upstream_connect_cb( evutil_socket_t s, short what, void *arg )
             return;
         } else if ( error ) {
             goto done;
-        } else if ( !upstream_init( s, conn->backend ) ) {
+        } else if ( upstream_init( s, conn->backend ) == NULL ) {
             goto done;
         }
         rc = LDAP_SUCCESS;
     }
 
 done:
+    LDAP_LIST_REMOVE( conn, next );
     if ( rc ) {
         evutil_closesocket( conn->fd );
         b->b_opening--;
@@ -72,18 +73,14 @@ done:
                     s, error ? ": " : "",
                     error ? sock_errstr( error, ebuf, sizeof(ebuf) ) : "" );
         }
+        backend_retry( b );
     } else {
         b->b_failed = 0;
     }
-    LDAP_LIST_REMOVE( conn, next );
     ldap_pvt_thread_mutex_unlock( &b->b_mutex );
 
     event_free( conn->event );
     ch_free( conn );
-
-    if ( rc ) {
-        backend_retry( b );
-    }
 }
 
 static void
@@ -168,7 +165,7 @@ upstream_name_cb( int result, struct evutil_addrinfo *res, void *arg )
         Debug( LDAP_DEBUG_CONNS, "upstream_name_cb: "
                 "connection to backend uri=%s in progress\n",
                 b->b_uri.bv_val );
-    } else if ( !upstream_init( s, b ) ) {
+    } else if ( upstream_init( s, b ) == NULL ) {
         goto fail;
     }
 
@@ -182,8 +179,8 @@ fail:
     }
     b->b_opening--;
     b->b_failed++;
-    ldap_pvt_thread_mutex_unlock( &b->b_mutex );
     backend_retry( b );
+    ldap_pvt_thread_mutex_unlock( &b->b_mutex );
     if ( res ) {
         evutil_freeaddrinfo( res );
     }
@@ -283,10 +280,14 @@ backend_select( LloadOperation *op, int *res )
     return NULL;
 }
 
+/*
+ * Will schedule a connection attempt if there is a need for it. Need exclusive
+ * access to backend, its b_mutex is not touched here, though.
+ */
 void
 backend_retry( LloadBackend *b )
 {
-    int rc, requested;
+    int requested;
 
     if ( slapd_shutdown ) {
         Debug( LDAP_DEBUG_CONNS, "backend_retry: "
@@ -294,8 +295,6 @@ backend_retry( LloadBackend *b )
         return;
     }
 
-    ldap_pvt_thread_mutex_lock( &b->b_mutex );
-
     requested = b->b_numconns;
 #ifdef LDAP_API_FEATURE_VERIFY_CREDENTIALS
     if ( !(lload_features & LLOAD_FEATURE_VC) )
@@ -303,38 +302,46 @@ backend_retry( LloadBackend *b )
     {
         requested += b->b_numbindconns;
     }
-    if ( b->b_active + b->b_bindavail + b->b_opening < requested ) {
-        if ( b->b_opening > 0 || b->b_failed > 0 ) {
-            if ( b->b_failed > 0 &&
-                    !event_pending( b->b_retry_event, EV_TIMEOUT, NULL ) ) {
-                Debug( LDAP_DEBUG_CONNS, "backend_retry: "
-                        "scheduling a retry in %d ms\n",
-                        b->b_retry_timeout );
-                b->b_opening++;
-                event_add( b->b_retry_event, &b->b_retry_tv );
-                ldap_pvt_thread_mutex_unlock( &b->b_mutex );
-                return;
-            } else {
-                Debug( LDAP_DEBUG_CONNS, "backend_retry: "
-                        "retry in progress already\n" );
-            }
-        } else {
-            Debug( LDAP_DEBUG_CONNS, "backend_retry: "
-                    "scheduling re-connection straight away\n" );
-            b->b_opening++;
-            rc = ldap_pvt_thread_pool_submit2(
-                    &connection_pool, backend_connect_task, b, &b->b_cookie );
-            if ( rc ) {
-                ldap_pvt_thread_mutex_unlock( &b->b_mutex );
-                backend_connect( -1, 0, b );
-                return;
-            }
-        }
-    } else {
+
+    if ( b->b_active + b->b_bindavail + b->b_opening >= requested ) {
         Debug( LDAP_DEBUG_CONNS, "backend_retry: "
                 "no more connections needed for this backend\n" );
+        return;
+    }
+
+    if ( b->b_opening > 0 ) {
+        Debug( LDAP_DEBUG_CONNS, "backend_retry: "
+                "retry in progress already\n" );
+        assert( b->b_opening == 1 );
+        return;
+    }
+
+    /* We incremented b_opening when we activated the event, so it can't be
+     * pending */
+    assert( !event_pending( b->b_retry_event, EV_TIMEOUT, NULL ) );
+    b->b_opening++;
+
+    if ( b->b_failed > 0 ) {
+        Debug( LDAP_DEBUG_CONNS, "backend_retry: "
+                "scheduling a retry in %d ms\n",
+                b->b_retry_timeout );
+        event_add( b->b_retry_event, &b->b_retry_tv );
+        return;
+    }
+
+    Debug( LDAP_DEBUG_CONNS, "backend_retry: "
+            "scheduling re-connection straight away\n" );
+
+    if ( ldap_pvt_thread_pool_submit2(
+                 &connection_pool, backend_connect_task, b, &b->b_cookie ) ) {
+        Debug( LDAP_DEBUG_ANY, "backend_retry: "
+                "failed to submit retry task, scheduling a retry instead\n" );
+        /* The current implementation of ldap_pvt_thread_pool_submit2 can fail
+         * and still set (an invalid) cookie */
+        b->b_cookie = NULL;
+        b->b_failed++;
+        event_add( b->b_retry_event, &b->b_retry_tv );
     }
-    ldap_pvt_thread_mutex_unlock( &b->b_mutex );
 }
 
 void
@@ -421,7 +428,7 @@ backend_connect( evutil_socket_t s, short what, void *arg )
             Debug( LDAP_DEBUG_CONNS, "backend_connect: "
                     "connection to backend uri=%s in progress\n",
                     b->b_uri.bv_val );
-        } else if ( !upstream_init( s, b ) ) {
+        } else if ( upstream_init( s, b ) == NULL ) {
             goto fail;
         }
 
@@ -466,8 +473,8 @@ backend_connect( evutil_socket_t s, short what, void *arg )
 fail:
     b->b_opening--;
     b->b_failed++;
-    ldap_pvt_thread_mutex_unlock( &b->b_mutex );
     backend_retry( b );
+    ldap_pvt_thread_mutex_unlock( &b->b_mutex );
 }
 
 void *

diff --git a/servers/lloadd/daemon.c b/servers/lloadd/daemon.c
index f0616c911d3f71ed3647f296f5752ff0cb8e8c0b..4deb5458720c681dbea4dc0d52b06706dd2a377e 100644 (file)
--- a/servers/lloadd/daemon.c
+++ b/servers/lloadd/daemon.c
@@ -1354,9 +1354,11 @@ lloadd_daemon( struct event_base *daemon_base )
                         "failed to allocate retry event\n" );
                 return -1;
             }
-            b->b_retry_event = event;
 
+            ldap_pvt_thread_mutex_lock( &b->b_mutex );
+            b->b_retry_event = event;
             backend_retry( b );
+            ldap_pvt_thread_mutex_unlock( &b->b_mutex );
         }
     }
 

diff --git a/servers/lloadd/upstream.c b/servers/lloadd/upstream.c
index 77fc3fd490d89210c300c21c2b0efddfcce16afa..d883a5564aaad117b1514bee2dca8b5abd99df67 100644 (file)
--- a/servers/lloadd/upstream.c
+++ b/servers/lloadd/upstream.c
@@ -330,8 +330,8 @@ upstream_bind_cb( LloadConnection *c )
                 LDAP_CIRCLEQ_INSERT_HEAD( &b->b_conns, c, c_next );
             }
             b->b_last_conn = c;
-            ldap_pvt_thread_mutex_unlock( &b->b_mutex );
             backend_retry( b );
+            ldap_pvt_thread_mutex_unlock( &b->b_mutex );
             CONNECTION_LOCK_DECREF(c);
         } break;
 #ifdef HAVE_CYRUS_SASL
@@ -410,7 +410,7 @@ static int
 upstream_finish( LloadConnection *c )
 {
     LloadBackend *b = c->c_private;
-    int is_bindconn = 0, rc = 0;
+    int is_bindconn = 0;
 
     c->c_pdu_cb = handle_one_response;
 
@@ -459,16 +459,28 @@ upstream_finish( LloadConnection *c )
         }
         b->b_last_conn = c;
     } else {
-        rc = 1;
+        if ( ldap_pvt_thread_pool_submit(
+                     &connection_pool, upstream_bind, c ) ) {
+            Debug( LDAP_DEBUG_ANY, "upstream_finish: "
+                    "failed to set up a bind callback for connid=%lu\n",
+                    c->c_connid );
+            return -1;
+        }
         c->c_refcnt++;
-        ldap_pvt_thread_pool_submit( &connection_pool, upstream_bind, c );
+
+        Debug( LDAP_DEBUG_CONNS, "upstream_finish: "
+                "scheduled a bind callback for connid=%lu\n",
+                c->c_connid );
+        return LDAP_SUCCESS;
     }
 
     Debug( LDAP_DEBUG_CONNS, "upstream_finish: "
-            "%sconnection connid=%lu is%s ready for use\n",
-            is_bindconn ? "bind " : "", c->c_connid, rc ? " almost" : "" );
+            "%sconnection connid=%lu for backend server '%s' is ready for "
+            "use\n",
+            is_bindconn ? "bind " : "", c->c_connid, b->b_name.bv_val );
 
-    return rc;
+    backend_retry( b );
+    return LDAP_SUCCESS;
 }
 
 static void
@@ -521,11 +533,10 @@ upstream_tls_handshake_cb( evutil_socket_t s, short what, void *arg )
         CONNECTION_LOCK_DECREF(c);
 
         rc = upstream_finish( c );
-
         ldap_pvt_thread_mutex_unlock( &b->b_mutex );
 
-        if ( rc == LDAP_SUCCESS ) {
-            backend_retry( b );
+        if ( rc ) {
+            goto fail;
         }
     } else if ( ber_sockbuf_ctrl( c->c_sb, LBER_SB_OPT_NEEDS_WRITE, NULL ) ) {
         event_add( c->c_write_event, lload_write_timeout );
@@ -604,21 +615,20 @@ upstream_starttls( LloadConnection *c )
         }
         c->c_is_tls = LLOAD_CLEARTEXT;
 
-        ber_free( ber, 1 );
-
         CONNECTION_UNLOCK_INCREF(c);
         ldap_pvt_thread_mutex_lock( &b->b_mutex );
         CONNECTION_LOCK_DECREF(c);
 
         rc = upstream_finish( c );
-
         ldap_pvt_thread_mutex_unlock( &b->b_mutex );
 
-        if ( rc == LDAP_SUCCESS ) {
-            backend_retry( b );
+        if ( rc ) {
+            goto fail;
         }
 
+        ber_free( ber, 1 );
         CONNECTION_UNLOCK_OR_DESTROY(c);
+
         return rc;
     }
 
@@ -656,7 +666,7 @@ upstream_init( ber_socket_t s, LloadBackend *b )
     LloadConnection *c;
     struct event_base *base = lload_get_base( s );
     struct event *event;
-    int flags, rc = -1;
+    int flags;
 
     assert( b != NULL );
 
@@ -695,8 +705,7 @@ upstream_init( ber_socket_t s, LloadBackend *b )
     c->c_write_event = event;
 
     if ( c->c_is_tls == LLOAD_CLEARTEXT ) {
-        rc = upstream_finish( c );
-        if ( rc < 0 ) {
+        if ( upstream_finish( c ) ) {
             goto fail;
         }
     } else if ( c->c_is_tls == LLOAD_LDAPS ) {
@@ -730,13 +739,6 @@ upstream_init( ber_socket_t s, LloadBackend *b )
     c->c_destroy = upstream_destroy;
     CONNECTION_UNLOCK_OR_DESTROY(c);
 
-    /* has upstream_finish() finished? */
-    if ( rc == LDAP_SUCCESS ) {
-        ldap_pvt_thread_mutex_unlock( &b->b_mutex );
-        backend_retry( b );
-        ldap_pvt_thread_mutex_lock( &b->b_mutex );
-    }
-
     return c;
 
 fail:
@@ -840,8 +842,8 @@ upstream_destroy( LloadConnection *c )
             b->b_active--;
         }
         b->b_n_ops_executing -= executing;
-        ldap_pvt_thread_mutex_unlock( &b->b_mutex );
         backend_retry( b );
+        ldap_pvt_thread_mutex_unlock( &b->b_mutex );
     }
 
     CONNECTION_LOCK_DECREF(c);