> On Dec 31, 2017, at 9:16 PM, Ben Pfaff <b...@ovn.org> wrote:

> +/* Sets the number of connection attempts that will be made without backoff 
> to
> + * 'backoff_free_tries'.  Values 0 and 1 both represent a single attempt. */
> +void
> +reconnect_set_backoff_free_tries(struct reconnect *fsm,
> +                                 unsigned int backoff_free_tries)
> +{
> +    fsm->backoff_free_tries = backoff_free_tries;
> +}

You're entitled to one freebie.

> @@ -346,7 +356,7 @@ reconnect_disconnected(struct reconnect *fsm, long long 
> int now, int error)
>                 VLOG(fsm->info, "%s: error listening for connections",
>                      fsm->name);
>             }
> -        } else {
> +        } else if (fsm->backoff < fsm->max_backoff) {
>             const char *type = fsm->passive ? "listen" : "connection";
>             if (error > 0) {
>                 VLOG_INFO("%s: %s attempt failed (%s)",

This check confused me until I got to the bottom and realized that it's 
purposely setting "backoff" to "max_backoff" to suppress log messages.  It 
might be worth clarifying that a bit, since otherwise, the test seems sort of 
backwards.

>         /* Back off. */
> -        if (fsm->state & (S_ACTIVE | S_IDLE)
> -             && (fsm->last_activity - fsm->last_connected >= fsm->backoff
> -                 || fsm->passive)) {
> +        if (fsm->backoff_free_tries > 1) {
> +            fsm->backoff_free_tries--;
> +            fsm->backoff = 0;
> +        } else if (fsm->state & (S_ACTIVE | S_IDLE)
> +                   && (fsm->last_activity - fsm->last_connected >= 
> fsm->backoff
> +                       || fsm->passive)) {
>             fsm->backoff = fsm->passive ? 0 : fsm->min_backoff;
>         } else {
>             if (fsm->backoff < fsm->min_backoff) {
>                 fsm->backoff = fsm->min_backoff;
> -            } else if (fsm->backoff >= fsm->max_backoff / 2) {
> -                fsm->backoff = fsm->max_backoff;
> -            } else {
> +            } else if (fsm->backoff < fsm->max_backoff / 2) {
>                 fsm->backoff *= 2;
> -            }
> -            if (fsm->passive) {
> -                VLOG(fsm->info, "%s: waiting %.3g seconds before trying to "
> -                          "listen again", fsm->name, fsm->backoff / 1000.0);
> +                VLOG(fsm->info, "%s: waiting %.3g seconds before %s",
> +                     fsm->name, fsm->backoff / 1000.0,
> +                     fsm->passive ? "trying to listen again" : "reconnect");
>             } else {
> -                VLOG(fsm->info, "%s: waiting %.3g seconds before reconnect",
> -                          fsm->name, fsm->backoff / 1000.0);
> +                if (fsm->backoff < fsm->max_backoff) {
> +                    VLOG_INFO("%s: continuing to %s in the background but "
> +                              "suppressing further logging", fsm->name,
> +                              fsm->passive ? "try to listen" : "reconnect");
> +                }
> +                fsm->backoff = fsm->max_backoff;
>             }
>         }

This logic for multiplying the backoff by 2 is different from the original 
code.  I think that's because the previous logic was wrong, but I just wanted 
to make sure.

Acked-by: Justin Pettit <jpet...@ovn.org>

--Justin


_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to