Patch to add timezone support for timeperiods

[John Lemon]

Andreas Ericsson wrote:

> Honorable intentions, and very nice if it works properly. I'm not sure
doing
> it the way you have is the best way to go about it though. For
example, I
> could very well see timezone as something one would rather want to set
for
> objects using timeperiods (such as hosts, services, contacts...), and
then
> simply creating meta-timeperiods with pre-recalculated values. I
appreciate
> the fact that such a patch would be quite a lot more complex, but if
the
> feature is to be introduced, we might as well do it properly from the
start
> and save ourselves the feature requests later ;-)

Looked at doing it that way at first and yes it is more complex. But
since the values are all pre-calculated and static, this would not
handle changes in timezone transitions from/to daylight savings  time on
the fly. (which is our greatest pain as governments keep changing the
rules for their timezones). 

So I guess I am after more then just converting timeperiods with
timezone definitions into the equivalent local (to the nagios server
host's) timeperiods.  

> I'm also rather skeptical to the free use of tzset(), especially since
Nagios
> is a threaded application. What happens if one thread wants to log or
schedule
> something while another thread has TZ set to something completely
bonkers?
> Will the log get corrupted (= very bad)? Will the scheduled event be
scheduled
> several hours into the future, or perhaps even in the past?

> I'm not against the functionality, but I fear there may be subtle
errors that
> turn out to be extremely hard to debug with the approach you've taken,
and would
> much prefer timeperiods to be modified when read from configuration
than in-core
> while Nagios is running.

Not sure on this but isn't the command_file_worker_thread the only other
thread? (modules?) ?

But yes the code is not thread same. I saw that localtime was used in
check_time_against_period etc which is not thread safe with some OS's
and just followed the flow (i.e. assuming it was not threaded)  

So something like the below for the check_time_against_period function
would work safely in a thread. (this would only be executed if a
timezone was defined for a timeperiod) 

             /*place thread_mutex lock here for tz change*/
                set_environment_var("TZ",tperiod->timezone,1);
                tzset();
                /* convert test_time to tm structure in new TZ
timezone*/
                localtime_r((time_t *)&test_time, &tz_t);
 
set_environment_var("TZ",current_timezone,current_timezone?1:0);
                tzset();
             /*thread unlock*/
                /* convert tz_t structure (based on new TZ) back to
local TZ time*/
                test_time = mktime(&tz_t);
               

Doing it this way is safer for threads and removes all the tzset apart
from the two, protected by a lock.

Also for the get_next_valid_time I can now just calculate the timezone
offset in seconds

             /*place thread_mutex lock here for tz change*/
                set_environment_var("TZ",tperiod->timezone,1);
                tzset();
                /* convert test_time to tm structure in new TZ
timezone*/
                localtime_r((time_t *)&pref_time, &tz_t);
 
set_environment_var("TZ",current_timezone,current_timezone?1:0);
                tzset();
             /*thread unlock*/
                /* convert tz_t structure (based on new TZ) back to
local TZ time*/
                diff = mktime(&tz_t) - pref_time;

then add the diff (offset) before doing the time checks and remove it
before returning the new valid_time 

Will test these changes out and check if the locking causing any
performance issues.

So back to more testing. 

Thanks
John 

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/