From: | Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com> |
---|---|
To: | Noah Misch <noah(at)leadboat(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: stats_temp_directory conflicts |
Date: | 2018-12-31 21:52:33 |
Message-ID: | e0fb20e7-1654-baf1-5143-00eb941e8f56@2ndQuadrant.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 12/30/18 11:06 PM, Noah Misch wrote:
> On Sun, Dec 30, 2018 at 07:47:05PM -0500, Tom Lane wrote:
>> Noah Misch <noah(at)leadboat(dot)com> writes:
>>> I'm thinking the server should manage this; during startup, create
>>> $stats_temp_directory/PostgreSQL.$postmaster_pid and store each stats file
>>> therein.
>> +1
>>
>>> Just before creating that directory, scan $stats_temp_directory and
>>> delete subdirectories that no longer correspond to live PIDs.
>> Hm, seems potentially racy, if multiple postmasters are starting
>> at the same time. The only one you can be *sure* is dead is one
>> with your own PID.
> True; if a PID=123 postmaster launches and completes startup after a
> slightly-older PID=122 postmaster issues kill(123, 0) and before PID=122
> issues unlink()s, PID=122 unlinks files wrongly. I think I would fix that
> with fcntl(F_SETLKW)/Windows-equivalent on some file in stats_temp_directory.
> One would acquire the lock before deleting a subdirectory, except for a
> postmaster deleting the directory it created. (If a postmaster finds a stale
> directory for its PID, delete that directory and create a new one. Like any
> deletion, one must hold the lock.)
>
> (Alternately, one could just accept that risk.)
>
> Another problem comes to mind; if postmasters of different UIDs share a
> stats_temp_directory, then a PID=1234 postmaster may find itself unable to
> delete the stale PostgreSQL.1234 subdirectory owned by some other UID. To fix
> that, the name pattern probably should be
> $stats_temp_directory/PostgreSQL.$euid.$postmaster_pid.
I like this scheme. It will certainly make using a RAMdisk simpler for
buildfarm members.
+1 for locking rather than running the risk of incorrect deletions.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From | Date | Subject | |
---|---|---|---|
Next Message | Justin Pryzby | 2018-12-31 22:17:34 | pg11.1: dsa_area could not attach to segment |
Previous Message | Andres Freund | 2018-12-31 21:41:33 | Re: [HACKERS] REINDEX CONCURRENTLY 2.0 |