On Thu, May 5, 2022 at 12:11 PM Bharath Rupireddy
<bharath.rupireddyforpostg...@gmail.com> wrote:
>
> On Mon, May 2, 2022 at 6:26 PM Ashutosh Bapat
> <ashutosh.bapat....@gmail.com> wrote:
> >
> > Hi Bharath,
> >
> >
> > On Sat, Apr 30, 2022 at 11:08 AM Bharath Rupireddy
> > <bharath.rupireddyforpostg...@gmail.com> wrote:
> > >
> > > Hi,
> > >
> > > At times, there can be many temp files (under pgsql_tmp) and temp
> > > relation files (under removal which after crash may take longer during
> > > which users have no clue about what's going on in the server before it
> > > comes up online.
> > >
> > > Here's a proposal to use ereport_startup_progress to report the
> > > progress of the file removal.
> > >
> > > Thoughts?
> >
> > The patch looks good to me.
> >
> > With this patch, the user would at least know which directory is being
> > scanned and how much time has elapsed.
>
> There's a problem with the patch, the timeout mechanism isn't being
> used by the postmaster process. Postmaster doesn't
> InitializeTimeouts() and doesn't register STARTUP_PROGRESS_TIMEOUT, I
> tried to make postmaster do that (attached a v2 patch) but make check
> fails.
>
> Now, I'm thinking if it's a good idea to let postmaster use timeouts at all?
Here's the v3 patch, which adds progress reports for temp file removal
under the pgsql_tmp directory and temporary relation files under the
pg_tblspc directory, regression tests pass with it.
Regards,
Bharath Rupireddy.
From c31492191ffcd6222d7e42917cbd928351623780 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: Tue, 2 Aug 2022 05:10:06 +0000
Subject: [PATCH v3] Progress report removal of temp files and temp relation
files
At times, there can be many temp files (under pgsql_tmp) and temp
relation files (under removal which after crash may take longer
during which users have no clue about what's going on in the
server before it comes up online. This patch uses
ereport_startup_progress to report the progress of the file
removal.
---
src/backend/postmaster/postmaster.c | 10 +++++++++-
src/backend/storage/file/fd.c | 13 +++++++++++++
src/backend/utils/misc/timeout.c | 11 +++++++++--
3 files changed, 31 insertions(+), 3 deletions(-)
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index e541b16bdb..40b53276be 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -116,6 +116,7 @@
#include "postmaster/interrupt.h"
#include "postmaster/pgarch.h"
#include "postmaster/postmaster.h"
+#include "postmaster/startup.h"
#include "postmaster/syslogger.h"
#include "replication/logicallauncher.h"
#include "replication/walsender.h"
@@ -653,7 +654,6 @@ PostmasterMain(int argc, char *argv[])
pqsignal_pm(SIGINT, pmdie); /* send SIGTERM and shut down */
pqsignal_pm(SIGQUIT, pmdie); /* send SIGQUIT and die */
pqsignal_pm(SIGTERM, pmdie); /* wait for children and shut down */
- pqsignal_pm(SIGALRM, SIG_IGN); /* ignored */
pqsignal_pm(SIGPIPE, SIG_IGN); /* ignored */
pqsignal_pm(SIGUSR1, sigusr1_handler); /* message from child process */
pqsignal_pm(SIGUSR2, dummy_handler); /* unused, reserve for children */
@@ -688,6 +688,11 @@ PostmasterMain(int argc, char *argv[])
pqsignal_pm(SIGXFSZ, SIG_IGN); /* ignored */
#endif
+ InitializeTimeouts(); /* establishes SIGALRM handler */
+
+ RegisterTimeout(STARTUP_PROGRESS_TIMEOUT,
+ startup_progress_timeout_handler);
+
/*
* Options setup
*/
@@ -1119,6 +1124,9 @@ PostmasterMain(int argc, char *argv[])
/* Write out nondefault GUC settings for child processes to use */
write_nondefault_variables(PGC_POSTMASTER);
+ /* Prepare to report progress of the temporary files removal phase */
+ begin_startup_progress_phase();
+
/*
* Clean out the temp directory used to transmit parameters to child
* processes (see internal_forkexec, below). We must do this before
diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index f904f60c08..37c13a60ba 100644
--- a/src/backend/storage/file/fd.c
+++ b/src/backend/storage/file/fd.c
@@ -102,6 +102,7 @@
#include "storage/ipc.h"
#include "utils/guc.h"
#include "utils/resowner_private.h"
+#include "utils/timeout.h"
/* Define PG_FLUSH_DATA_WORKS if we have an implementation for pg_flush_data */
#if defined(HAVE_SYNC_FILE_RANGE)
@@ -3093,6 +3094,12 @@ RemovePgTempFiles(void)
DIR *spc_dir;
struct dirent *spc_de;
+ /*
+ * Prepare to report progress of the temporary and temporary relation files
+ * removal phase.
+ */
+ begin_startup_progress_phase();
+
/*
* First process temp files in pg_default ($PGDATA/base)
*/
@@ -3166,6 +3173,9 @@ RemovePgTempFilesInDir(const char *tmpdirname, bool missing_ok, bool unlink_all)
snprintf(rm_path, sizeof(rm_path), "%s/%s",
tmpdirname, temp_de->d_name);
+ ereport_startup_progress("removing temporary files under pgsql_tmp directory, elapsed time: %ld.%02d s, current file: %s",
+ rm_path);
+
if (unlink_all ||
strncmp(temp_de->d_name,
PG_TEMP_FILE_PREFIX,
@@ -3256,6 +3266,9 @@ RemovePgTempRelationFilesInDbspace(const char *dbspacedirname)
snprintf(rm_path, sizeof(rm_path), "%s/%s",
dbspacedirname, de->d_name);
+ ereport_startup_progress("removing temporary relation files under pg_tblspc directory, elapsed time: %ld.%02d s, current file: %s",
+ rm_path);
+
if (unlink(rm_path) < 0)
ereport(LOG,
(errcode_for_file_access(),
diff --git a/src/backend/utils/misc/timeout.c b/src/backend/utils/misc/timeout.c
index 6f5e08bc30..cc4b9d6ad7 100644
--- a/src/backend/utils/misc/timeout.c
+++ b/src/backend/utils/misc/timeout.c
@@ -16,6 +16,7 @@
#include <sys/time.h>
+#include "libpq/pqsignal.h"
#include "miscadmin.h"
#include "storage/proc.h"
#include "utils/timeout.h"
@@ -375,8 +376,11 @@ handle_sig_alarm(SIGNAL_ARGS)
/*
* SIGALRM is always cause for waking anything waiting on the process
* latch.
+ *
+ * Postmaster has no latch associated with it.
*/
- SetLatch(MyLatch);
+ if (MyLatch)
+ SetLatch(MyLatch);
/*
* Always reset signal_pending, even if !alarm_enabled, since indeed no
@@ -494,7 +498,10 @@ InitializeTimeouts(void)
all_timeouts_initialized = true;
/* Now establish the signal handler */
- pqsignal(SIGALRM, handle_sig_alarm);
+ if (IsPostmasterEnvironment)
+ pqsignal_pm(SIGALRM, handle_sig_alarm);
+ else
+ pqsignal(SIGALRM, handle_sig_alarm);
}
/*
--
2.34.1
