Re: Confusing behavior of psql's \e

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>
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
Date: 2021-03-10 05:13:58
Message-ID: 2063932.1615353238@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at> writes:
> On Thu, 2021-03-04 at 16:51 +0000, Jacob Champion wrote:
>> You could backdate the temporary file, so that any save is guaranteed
>> to move the timestamp forward. That should work even if the filesystem
>> has extremely poor precision.

> Ah, of course, that is the way to go.

I took a quick look at this. I don't have an opinion yet about the
question of changing the when-to-discard-the-buffer rules, but 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

else
{
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"?

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bharath Rupireddy 2021-03-10 05:14:12 Logical Replication - improve error message while adding tables to the publication in check_publication_add_relation
Previous Message Masahiro Ikeda 2021-03-10 05:11:49 Re: About to add WAL write/fsync statistics to pg_stat_wal view