Re: [Nagios-devel] Bug fix priority
Posted: Sun Aug 16, 2009 2:31 pm
This is a multi-part message in MIME format.
--------------030209040803070003090902
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Ethan Galstad wrote:
> There have been a couple of bugs related to timeperiods and scheduling
> lately. One was fixed in yesterday's CVS commit.
>
> As far as I can tell, there is a big bug in check scheduling when a
> timeperiod is used that contains an "exclude" directive. The scheduling
> logic will incorrectly schedule the next check too far into the future,
> when earlier scheduling is possible.
>
> Proposed patches to this point have been incorrect, as the real solution
> is much more complex. I believe I know what needs to be done to solve
> the issue, but it will require some time to architect/code the right
> solution. I'll work on getting this fix into the 3.4 release and mark
> it as a "known issue" for the time being.
>
I've been trying to work on a fix for this, please let me know what you
think of the attached diff.
I've effectively left the get_next_valid_time function invocation as is,
and adjusted it to call a recursive function that will run through a
timeperiod and all its exclusions.
I've moved the code to get the next earliest time to a function called
get_next_valid_time_per_timeperiod, and have added a function called
get_min_invalid_time_per_timeperiod which gets the earliest end_time
from preferred time for a timeperiod.
since a given timeperiod is inclusive, and its excludes are exclusive,
but the excludes` excludes are inclusive (and so on), the recursive
function get_earliest_time bumps a variable called level before
processing each excludes, and this is used to determine if the
timeperiod in question is an include or exclude in the context.
depending on this, the two different functions are called, one fetching
the earliest time as per previous code (which may not be the earliest
possible time as you noted above), and one fetching the earliest
end_time of exclusions. the smallest value found is then used as the
next valid time.
I've done very basic testing only with weekdays (sunday 00:00-09:00 etc)
and with exclusions, and also so far with one type of daterange
exception (day 16 12:00-21:00) also with exclusions.
the configuration for the simple testing I was doing is attached. if you
are okay with it working this way, could we test all possible daterange
types, I'll be happy to do this.
let me know what you think of it, if it's acceptable, I'll happily
consider taking configurations from people using complex timeperiods to
test this further.
--------------030209040803070003090902
Content-Type: text/x-patch;
name="utils.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="utils.diff"
--- utils.c 2009-08-11 19:29:52.000000000 +0200
+++ /tmp/utils.c 2009-08-16 16:42:51.000000000 +0200
@@ -1081,6 +1081,67 @@
/* given a preferred time, get the next valid time within a time period */
void get_next_valid_time(time_t pref_time, time_t *valid_time, timeperiod *tperiod){
time_t preferred_time=(time_t)0L;
+ time_t current_time=(time_t)0L;
+
+ log_debug_info(DEBUGL_FUNCTIONS,0,"get_next_valid_time()\n");
+
+ /* get time right now, preferred time must be now or in the future */
+ time(¤t_time);
+ preferred_time=(pref_time<current_time)?current_time:pref_time;
+
+ /* if no timeperiod, go with preferred time */
+ if(tperiod==NULL){
+ *valid_time=preferred_time;
+ return;
+ }
+
+ /* if the preferred time is valid in timeperiod, go with it */
+ /* this is necessary because the code below won't catch exceptions where preferred day is last (or only) date in timeperiod (date range) and last valid time has already passed */
+ /* performing this check and bailing out early allows us to skip having to check the next instance of a date range exception or weekday to determine the next valid time */
+ if(check_time_against_period(preferred_time,tperiod)==OK){
+#ifdef TEST_TIMEPERIODS_B
+ printf("PREF TIME IS VALID\n");
+#endif
+ *valid_time=preferred_time;
+ return;
+ }
+
+ get_earliest_time(p
...[email truncated]...
This post was automatically imported from historical nagios-devel mailing list archives
Original poster: [email protected]
--------------030209040803070003090902
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Ethan Galstad wrote:
> There have been a couple of bugs related to timeperiods and scheduling
> lately. One was fixed in yesterday's CVS commit.
>
> As far as I can tell, there is a big bug in check scheduling when a
> timeperiod is used that contains an "exclude" directive. The scheduling
> logic will incorrectly schedule the next check too far into the future,
> when earlier scheduling is possible.
>
> Proposed patches to this point have been incorrect, as the real solution
> is much more complex. I believe I know what needs to be done to solve
> the issue, but it will require some time to architect/code the right
> solution. I'll work on getting this fix into the 3.4 release and mark
> it as a "known issue" for the time being.
>
I've been trying to work on a fix for this, please let me know what you
think of the attached diff.
I've effectively left the get_next_valid_time function invocation as is,
and adjusted it to call a recursive function that will run through a
timeperiod and all its exclusions.
I've moved the code to get the next earliest time to a function called
get_next_valid_time_per_timeperiod, and have added a function called
get_min_invalid_time_per_timeperiod which gets the earliest end_time
from preferred time for a timeperiod.
since a given timeperiod is inclusive, and its excludes are exclusive,
but the excludes` excludes are inclusive (and so on), the recursive
function get_earliest_time bumps a variable called level before
processing each excludes, and this is used to determine if the
timeperiod in question is an include or exclude in the context.
depending on this, the two different functions are called, one fetching
the earliest time as per previous code (which may not be the earliest
possible time as you noted above), and one fetching the earliest
end_time of exclusions. the smallest value found is then used as the
next valid time.
I've done very basic testing only with weekdays (sunday 00:00-09:00 etc)
and with exclusions, and also so far with one type of daterange
exception (day 16 12:00-21:00) also with exclusions.
the configuration for the simple testing I was doing is attached. if you
are okay with it working this way, could we test all possible daterange
types, I'll be happy to do this.
let me know what you think of it, if it's acceptable, I'll happily
consider taking configurations from people using complex timeperiods to
test this further.
--------------030209040803070003090902
Content-Type: text/x-patch;
name="utils.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="utils.diff"
--- utils.c 2009-08-11 19:29:52.000000000 +0200
+++ /tmp/utils.c 2009-08-16 16:42:51.000000000 +0200
@@ -1081,6 +1081,67 @@
/* given a preferred time, get the next valid time within a time period */
void get_next_valid_time(time_t pref_time, time_t *valid_time, timeperiod *tperiod){
time_t preferred_time=(time_t)0L;
+ time_t current_time=(time_t)0L;
+
+ log_debug_info(DEBUGL_FUNCTIONS,0,"get_next_valid_time()\n");
+
+ /* get time right now, preferred time must be now or in the future */
+ time(¤t_time);
+ preferred_time=(pref_time<current_time)?current_time:pref_time;
+
+ /* if no timeperiod, go with preferred time */
+ if(tperiod==NULL){
+ *valid_time=preferred_time;
+ return;
+ }
+
+ /* if the preferred time is valid in timeperiod, go with it */
+ /* this is necessary because the code below won't catch exceptions where preferred day is last (or only) date in timeperiod (date range) and last valid time has already passed */
+ /* performing this check and bailing out early allows us to skip having to check the next instance of a date range exception or weekday to determine the next valid time */
+ if(check_time_against_period(preferred_time,tperiod)==OK){
+#ifdef TEST_TIMEPERIODS_B
+ printf("PREF TIME IS VALID\n");
+#endif
+ *valid_time=preferred_time;
+ return;
+ }
+
+ get_earliest_time(p
...[email truncated]...
This post was automatically imported from historical nagios-devel mailing list archives
Original poster: [email protected]