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
commit 417581b287f4060178595cd96465adc639639290
Author: anastasia <a.lubennik...@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.c
index 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.c
index 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)
