On Fri, Jan 12, 2018 at 02:27:37PM -0800, Justin Pettit wrote:
> On Dec 31, 2017, at 9:16 PM, Ben Pfaff <b...@ovn.org> wrote:
> > @@ -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.

OK, I added an appropriate comment.

> >         } 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.

I'm really adding logic to suppress excessive logging here.  It follows
the similar implementation in rconn.c.

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

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

Reply via email to