NA

Fixed in development branch svn://svn.zabbix.com/branches/dev/ZBX-7326

(1) After "next:" label, there is the following code:

if (0 != escalation.escalationid && escalation.nextcheck < *nextcheck)
	*nextcheck = escalation.nextcheck;

We jump here without prior update to escalation.nextcheck in case escalation.status is ESCALATION_STATUS_COMPLETED and escalation is about to be deleted.

wiper RESOLVED in r40387

asaveljevs Suppose we have an active escalation, but in execute_escalation() we do not do anything for some reason. For instance, check_escalation() fails. In that case, escalation.nextcheck is not updated either.

asaveljevs Perhaps we should modify that "if" not to take completed escalations into account. REOPENED.

wiper right, RESOLVED in r40455

asaveljevs Suppose we have an active escalation. It executes the last escalation step and becomes a sleeping escalation. In this case, nextcheck is not updated, so escalator does not sleep and rechecks that (now sleeping) escalation immediately. If escalation turns from active to sleeping, we might wish to check it after a minute, same as for regular sleeping escalations. REOPENED.

wiper Scheduled escalation nextcheck when changing status to 'sleep'. RESOLVED in r40484

asaveljevs Looks good now. Can be closed after r40487 is looked at.

wiper reviewed, CLOSED

(2) In main_escalator_loop(), there is the following code:

sleeptime = nextcheck - now;
if (CONFIG_ESCALATOR_FREQUENCY < sleeptime)
	sleeptime = CONFIG_ESCALATOR_FREQUENCY;

The first line is incorrect, because process_escalations() functions can take a long time to execute, so now might be well in the past.

The conditional beginning in the second line can never be true, because in process_escalation() nextcheck is initialized to "*nextcheck = now + CONFIG_ESCALATOR_FREQUENCY;" and after that it can only decrease.

asaveljevs You might also with to consider using calculate_sleeptime() function, same as in poller.c.

wiper RESOLVED in r40384, r40386

asaveljevs CLOSED

(3) Why did you implement sleeping differently than in other places (i.e., why did you introduce show_stats variable)?

wiper
Yes, I know it's different. I think this implementation is simpler, too bad it did not came to my mind when I was reviewing the statistics code in other places. Actually I could not decide which would be better - to keep in the same style as the majority of processes or have this version.

wiper refactored escalator main loop to be more similar to the other processes r40389
RESOLVED

asaveljevs Looks good, but see also r40399. RESOLVED.

wiper reviewed, CLOSED.

Released in:
pre-2.2.2rc1 r40490
pre-2.3.0 r40491