--On Monday, June 28, 2004 3:36 PM -0400 Jim Trocki <[EMAIL PROTECTED]> wrote:
On Mon, 28 Jun 2004, David Nolan wrote:
While it doesn't add any bugs, I don't believe it fixes any either.
it does indeed fix the bug where a received would not reset the _trap_timer, preventing traptimeout from working at all. i've tested it and it works properly now.
trap timeouts work already. I get them on occasion.
Careful reading of the code makes it clear that _trap_timer is only ever relevant after a timeout has already occurred. It prevents a timeout alert from happening on every pass through the code.
that is not true. _trap_timer is what counts down timeout counter in the first place. it is what gauges whether or not a timeout has occurred. once a timeout happens, as indicated when _trap_timer drops to zero or below, is that do_alert is called and _trap_timer is then reset to the value of traptimeout, and it starts counting down again.
what's supposed to prevent _trap_timer from hitting 0 in the first place is the reception of a trap, and that is what was broken, and the patch i posted fixes that.
Here's the code that actually decides whether or not to call handle_trap_timeout:
if ($sref->{"_trap_timer"} <= 0 && $tm - $sref->{"_last_trap"} >
$sref->{"traptimeout"}) { $sref->{"_trap_timer"} = $sref->{"traptimeout"};
handle_trap_timeout ($group, $service);
}(This is from the CVS head version, the mon-1-0-pre* version uses
_last_uptrap, not _last_trap. IIRC I decided that was a logic bug and fixed it in my code. But thats a different issue.)
Note the second half of the if clause. The if clause is confusing, so I'll re-order it and put some parens in:
if (($tm - $sref->{"_last_trap"}) > $sref->{"traptimeout"})
&& ($sref->{"_trap_timer"} <= 0)) {
$sref->{"_trap_timer"} = $sref->{"traptimeout"};
handle_trap_timeout ($group, $service);
}So there are two clauses. One is testing whether we've recieved a trap
within the traptimeout window. The other test is checking whether _trap_timer is set. And since the only code that ever resets _trap_timer is inside the if statement, the only reason it wouldn't be less than zero is if a trap timeout has fired recently, or we're in the "Mon just started recently" state.
Again, I believe the only "bug" here is that the code is confusing. (Either its confusing both you and Tim, or its confusing me. I believe its you. :) Either _trap_timer should be made the only thing that controls timeouts (apply the patch to reset on each trap, and remove the second clause of the existing if statement) or it should be removed and replaced with the _last_traptimeout style code as I suggested earlier. Either way is acceptable to me.
-David
David Nolan <*> [EMAIL PROTECTED]
curses: May you be forced to grep the termcap of an unclean yacc while
a herd of rogue emacs fsck your troff and vgrind your pathalias!
_______________________________________________ mon mailing list [EMAIL PROTECTED] http://linux.kernel.org/mailman/listinfo/mon
