From: Anastasia Lubennikova <a.lubennikova@postgrespro.ru>
To: Michael Paquier <michael@paquier.xyz>
To: Euler Taveira <euler.taveira@2ndquadrant.com>
Cc: Tomas Vondra <tomas.vondra@2ndquadrant.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Subject: Re: cleanup temporary files after crash
Date: Thu, 26 Nov 2020 11:48:46 +0300
Message-ID: <1982ee3c-54c3-280a-806a-fd28ad8f19e6@postgrespro.ru> (raw)
In-Reply-To: <20201101012555.GC3000@paquier.xyz>
References: <CAH503wDKdYzyq7U-QJqGn=Gm6XmoK+6_6xTJ-Yn5WSvoHLY1Ww@mail.gmail.com>
<20201028184252.5vpoe65kcfebbmmf@development>
<CAH503wCg+yne7hKAAFFHsVPe+tPb-aF5VGaWG0ETqyB3GP=Q+g@mail.gmail.com>
<20201101012555.GC3000@paquier.xyz>
On 01.11.2020 04:25, Michael Paquier wrote:
> On Sat, Oct 31, 2020 at 09:01:15PM -0300, Euler Taveira wrote:
>> I thought about not providing a GUC at all or provide it in the developer
>> section. I've never heard someone saying that they use those temporary
>> files to investigate an issue. Regarding a crash, all information is already
>> available and temporary files don't provide extra details. This new
>> GUC is just to keep the previous behavior. I'm fine without the GUC, though.
> The original behavior is as old as 4a5f38c4, and last time we talked
> about that there were arguments about still keeping the existing
> behavior to not cleanup files during a restart-after-crash scenario
> for the sake of being useful just "in case". I have never used that
> property myself, TBH, and I have seen much more cases of users caring
> about the data folder not facing an ENOSPC particularly if they don't
> use different partitions for pg_wal/ and the main data folder.
> --
> Michael
Thank you, Euler for submitting this.
+1 for the feature. One of the platforms we support uses temp files a
lot and we faced the problem, while never actually used these orphan
files for debugging purposes.
I also think that the GUC is not needed here. This 'feature' was
internal from the very beginning, so users shouldn't care about
preserving old behavior. Without the GUC the patch is very simple,
please see attached version. I also omit the test, because I am not sure
it will be stable given that the RemovePgTempFiles() allows the
possibility of failure.
--
Anastasia Lubennikova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
Attachments:0001_remove_temporary_files_after_crash.patch (text/x-patch, 2.4K) download
commit 417581b287f4060178595cd96465adc639639290
Author: anastasia <a.lubennikova@postgrespro.ru>
Date: Thu Nov 26 11:45:05 2020 +0300
Remove temporary files after a backend crash
Successive crashes could result in useless storage usage until service
is restarted. It could be the case on host with inadequate resources.
This manual intervention for some environments is not desirable.
The only reason we kept temp files was that someone might want to examine
them for debugging purposes. With this patch we favor service continuity
over debugging.
Author: Euler Taveira
Reviewer: Anastasia Lubennikova
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.cindex b7799ed1d24..a151250734f 100644--- a/src/backend/postmaster/postmaster.c+++ b/src/backend/postmaster/postmaster.c@@ -3999,6 +3999,9 @@ PostmasterStateMachine(void)
ereport(LOG,
(errmsg("all server processes terminated; reinitializing")));
+ /* remove leftover temporary files after a crash */+ RemovePgTempFiles();+
/* allow background workers to immediately restart */
ResetBackgroundWorkerCrashTimes();
diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.cindex 05abcf72d68..d30bfc28541 100644--- a/src/backend/storage/file/fd.c+++ b/src/backend/storage/file/fd.c@@ -2992,15 +2992,15 @@ CleanupTempFiles(bool isCommit, bool isProcExit)
* remove any leftover files created by OpenTemporaryFile and any leftover
* temporary relation files created by mdcreate.
*
- * NOTE: we could, but don't, call this during a post-backend-crash restart- * cycle. The argument for not doing it is that someone might want to examine- * the temp files for debugging purposes. This does however mean that- * OpenTemporaryFile had better allow for collision with an existing temp- * file name.+ * This is also called during a post-backend-crash restart cycle. The argument+ * for not doing it is that crashes of queries using temp files can result in+ * useless storage usage that can only be reclaimed by a service restart.
*
* NOTE: this function and its subroutines generally report syscall failures
* with ereport(LOG) and keep going. Removing temp files is not so critical
* that we should fail to start the database when we can't do it.
+ * Because this routine is not strict, OpenTemporaryFile allows for collision+ * with an existing temp file name.
*/
void
RemovePgTempFiles(void)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: a.lubennikova@postgrespro.ru, michael@paquier.xyz, euler.taveira@2ndquadrant.com, tomas.vondra@2ndquadrant.com, pgsql-hackers@lists.postgresql.org, tgl@sss.pgh.pa.us
Subject: Re: cleanup temporary files after crash
In-Reply-To: <1982ee3c-54c3-280a-806a-fd28ad8f19e6@postgrespro.ru>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox