|From:||Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>|
|To:||Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>|
|Cc:||Jacob Champion <pchampion(at)vmware(dot)com>, "chap(at)anastigmatix(dot)net" <chap(at)anastigmatix(dot)net>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>|
|Subject:||Re: Confusing behavior of psql's \e|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
On Wed, 2021-03-10 at 00:13 -0500, Tom Lane wrote:
> I agree
> that trying to get rid of the race condition inherent in the existing
> file mtime test would be a good idea. However, I've got some
> portability-related gripes about how you are doing the latter:
> 1. There is no principled reason to assume that the epoch date is in the
> past. IIRC, Postgres' timestamp epoch of 2000-01-01 was in the future
> at the time we set it. More relevant to the immediate issue, I clearly
> recall a discussion at Red Hat in which one of the principal glibc
> maintainers (likely Ulrich Drepper, though I'm not quite sure) argued
> that 32-bit time_t could be used indefinitely by redefining the epoch
> forward by 2^32 seconds every often; which would require intervals of
> circa 68 years in which time_t was seen as a negative offset from a
> future epoch date, rather than an unsigned offset from a past date.
> Now, I thought he was nuts then and I still think that 32-bit hardware
> will be ancient history by 2038 ... but there may be systems that do it
> like that. glibc hates ABI breakage.
> 2. Putting an enormously old date on a file that was just created will
> greatly confuse onlookers, some of whom (such as backup or antivirus
> daemons) might not react pleasantly.
> Between #1 and #2, it's clearly worth the extra one or two lines of
> code to set the file dates to, say, "time(NULL) - 1", rather than
> assuming that zero is good enough.
> 3. I wonder about the portability of utime(2). I see that we are using
> it to cause updates of socket and lock file times, but our expectations
> for it there are rock-bottom low. I think that failing the edit command
> if utime() fails is an overreaction even with optimistic assumptions about
> its reliability. Doing that makes things strictly worse than simply not
> doing anything, because 99% of the time this refinement is unnecessary.
> In short, I think the relevant code ought to be more like
> struct utimbuf ut;
> ut.modtime = ut.actime = time(NULL) - 1;
> (void) utime(fname, &ut);
> (plus some comments of course)
> regards, tom lane
> PS: I seem to recall that some Microsoft filesystems have 2-second
> resolution on file mod times, so maybe it needs to be "time(NULL) - 2"?
Thanks for taking a look!
Taken together, these arguments are convincing.
Done like that in the attached patch version 4.
|Next Message||vignesh C||2021-03-10 10:33:38||Do we support upgrade of logical replication?|
|Previous Message||Amit Kapila||2021-03-10 10:18:47||Re: [PATCH] Provide more information to filter_prepare|