Add an option to ctime and friends to track /etc/localtime.

[julian]

Summary

Allow ctime (and friends) to notice when the timezone
link (/etc/localtime) has been changed.
This is useful when long running processes (e.g. in appliances)
are puting out data with times while the adminstrator changes thes
timezone of the machine. The alternative is to restart every daemon
on the machine that uses this service. In our appliance we default
to noticing but I plan to commit it with a default of "same
behaviour as prior". An environent variable can be set while running
a program t make it notice changes (or not).
If not set the default applies.

Documentationchanges will follow

Test Plan

In testing currently

[julian]

contrib/tzcode/stdtime/localtime.c

296

I would commit this a 0.. we use it as 1 at $JOB as we always want everything to notice when /etc/localtime changes.

[julian]

contrib/tzcode/stdtime/localtime.c

325

this bit may change if I can wrap my head around what lcl_is_set is trying to do

[julian]

contrib/tzcode/stdtime/localtime.c

240

nominal defaults. don't do the work every time you get called.. that could be a lot. Maybe once a minute or every 100 messages (which ever happens first) would be enough.
I'm unhappy about doing a time() each time, but maybe we can pass it down from callers if they already have it?

[imp]

Mostly FreeBSD project normal form changes needed to this code.

However, there's one error. It should be checking the TZ that's in use, not /etc/timezone which is merely the default (and is a link to the real timezone).

contrib/tzcode/stdtime/localtime.c

286

extra space here and other places, and those extra spaces aren't file wide, so please clean up to KNF.

295–302

This is the long way to say

watch_changes = (env_ptr = getenv("ZONEINFO_WATCH_CHANGES") && strncmp(env_ptr, "yes", 3) ==0)

298

Add a != 0 here because of strcmp interface.

306

what does this accomplish? Seems totally redundant.

308

This is wrong.
You should default to TZ_FILENAME if no TZ variable is set, otherwise you should watch the file associated with the TZ variable.

[imp]

I'm curious why you don't setup a kqueue event structure to point to the file and then just poll it each time through. That would be no more expensive than the time() call, and you would have no lags. Would also save all the stat calls too, which would be a win.

contrib/tzcode/stdtime/localtime.c

240

This seems tailor made for a kqueue thing...

[imp]

Sorry, ticked the wrong box.

[julian]

Despite my grumpy responses (Get off my lawn!) I appreciate the comments.. At least you didn't say "What a stupid idea, who would want to do that?" Anyhow you've given me some things to think about which is the aim. right?

contrib/tzcode/stdtime/localtime.c

240

do you think adding a kqueue infrastructure is worth it?

if you are in syslogd then yes you are running a long time
but what of the program that calls ctime exactly once.

I guess you could make it only do it if the user has bothered to set the env variable. which suggests he plans on sticking around..

I'll have to think about that idea.

286

will have a cleanup pass..

295–302

Or you could say that watch_changes = (env_ptr = getenv("ZONEINFO_WATCH_CHANGES") && strncmp(env_ptr, "yes", 3) ==0)

is the unreadable way of saying it the other way..
The compiler will figure it out and I cant read it your way. Well I can but it takes more effort for no real gain.
You also lose the fact that the default is settable, one way or the other which is important because at $JOB we want it to default on where in FreeBSD we probably (I may be wrong) want to default to off if the env var is not set. If I am reading your line right (which is hard), the value if there is no env value is set to 0 which is the opposite of the code above. SO we'd have to have it rewritten in our private branch instead of just having a value of 1 or 0 defined some where..

306

you could say "only check once a minute, but what if the time is set backwards or forwards. a count of calls is a catch-all.. you could also say We have several hundred logs per second and don't want to wait a minute. I agree there's duplication but not redundancy.
I guess you cold look at the absolute value of the time delta and not worry about the count.. though in a perfect world I'd rather get rid of the call to time().. the call count is cheaper :-)

308

well that depends.
I'm not watching for someone updating the definition of America/Podunk, but for the symlink suddenly pointing somewhere else.
If the user has set his TZ to America/Podunk then he's made up his mind and he isn't going to decide to change it.. because he can't.
You can't change the envrironment after start up. You could watch the file if TZ was set, but that wasn't in my brief and I doubt that people would rewrite America/Podunk. On the other hand they might have a file "/usr/share/zoneinfo/OURTIMEZONE" and set TZ to that I guess. The brief here was to just watch the symlink because that's what the UI would change.

If you think it's important then it can be done.. we just don't need it, so it's extra.

[julian]

In D14608#306803, @imp wrote:

I'm curious why you don't setup a kqueue event structure to point to the file and then just poll it each time through. That would be no more expensive than the time() call, and you would have no lags. Would also save all the stat calls too, which would be a win.

this idea is growing on me.. I'll think about it a bit more.

[cem]

Ditto Warner — polling /etc/localtime is probably the wrong approach. Sure, you've gotten rid of stat(2) on every single ctime() invocation, but instead we clock_gettime(2) every time instead (and poll the file every 10 seconds). I would also suggest a kqueue/kevent monitor.

[bdrewery]

I haven't had a chance to really look at this but Isilon has done something similar - but it doesn't really handle /etc/localtime changing very well. It is more avoiding rereading it constantly in a long running program.

static time_t lcl_checktime;
static time_t lcl_mtime;
static ino64_t lcl_ino;

#define TZ_RELOAD_INTERVAL 3

/*
 * Reload timezone if it has changed. The old method was to always reload
 * on every localtime(3) call.
 */
int
isi_should_force_reload_tz()
{
        const char *name;
        struct stat sb;
        int ret;
        struct timespec now;

        /*
         * Only check if the file has changed to avoid spamming syscalls in
         * syslog(3).
         */
        if (clock_gettime(CLOCK_SECOND, &now) != 0)
                return (1);
        /*
         * Only re-stat every TZ_RELOAD_INTERVAL seconds if not running
         * a configuration program.  Check getenv(3).
         */
        if (getenv("ISI_TIME_CONFIG") == NULL &&
            now.tv_sec - lcl_checktime <= TZ_RELOAD_INTERVAL)
                return (0);
        lcl_checktime = now.tv_sec;
        if ((name = getenv("TZ")) == NULL)
                name = TZDEFAULT;
        if (stat(name, &sb) != 0)
                return (1);
        if (sb.st_mtime != lcl_mtime || sb.st_ino != lcl_ino)
                ret = 1;
        else
                ret = 0;
        lcl_mtime = sb.st_mtime;
        lcl_ino = sb.st_ino;

        return (ret);
}

...

struct tm *
localtime(const time_t *const timep)
{
        struct tm *p_tm;

        if (isi_should_force_reload_tz())
                lcl_is_set = 0;

Same for localtime_r.

[cgull_glup.org]

contrib/tzcode/stdtime/localtime.c

308

Well, what if the timezone files themselves change? freebsd-update might do that without a reboot, for instance. That might be another case you want to handle, though I think it makes monitoring more complex, because then you have to watch both the symlink and its target.

I don't know that this is a must-do but it does seem a little inconsistent to update on symlink changes but not on file changes.

(I work for another appliance vendor, but I don't think this actually is an issue for us, so this is a somewhat theoretical observation)