Code review comment for lp:~pwlars/lava-dispatcher/move-network-checks

On Wed, Mar 02, 2011, Paul Larson wrote:
> > > > * I'm not entirely comfortable with using expect; I'm not sure how we can
> > make
> > > > sure the ping process has been created; of course ping is relatively
> > harmless,
> > > > but it seems this could result in 20 ping commands being queued; do we
> > need to
> > > > pass some timeout?
> > > We do, in wait_network_up, which calls check_network_up
> >
> > What I mean is that if you expect timeout is 2 seconds and your ping
> > times out only after 10 seconds, then that loop in wait_network_up()
> > might launch 5 pings (since expect will timeout before the ping does)
> > before the first one finally returns, and then your console will be
> > running these other pings when you are trying to run another command
> > etc.
> But I think I addressed this already by setting the ping timeout to 4
> seconds and the expect timeout to 5. So if it passes
> check_network_up(), then it will come back right away most likely, and
> go through without another ping command being sent. If it fails the
> ping, it won't return from check_network_up() for 5 seconds, after
> which another one will be sent. I don't see where command stacking
> could occur here.

 Sure; since your comment was on my original review, I thought you meant
 it wasn't an issue in the first place; with the 4 / 5 seconds timeouts,
 it's much better. Still, there's room for ping not to timeout despite
 being told to, which is why I was suggesting that any expect timeout
 should be a fatal error. Anyway, the sooner we move away from expect,
 the better :-)

--
Loïc Minier