public inbox for [email protected]
help / color / mirror / Atom feedFrom: Hayato Kuroda (Fujitsu) <[email protected]>
To: 'Michael Paquier' <[email protected]>
Cc: 'PostgreSQL Hackers' <[email protected]>
Subject: RE: pg_ctl start may return 0 even if the postmaster has been already started on Windows
Date: Thu, 7 Sep 2023 10:53:41 +0000
Message-ID: <TYAPR01MB58662D7A48E8587A14244097F5EEA@TYAPR01MB5866.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <[email protected]>
References: <TYAPR01MB586654E2D74B838021BE77CAF5EEA@TYAPR01MB5866.jpnprd01.prod.outlook.com>
<[email protected]>
Dear Michael,
Thank you for replying!
> Not failing on `pg_ctl start` if the command is run on a data folder
> that has already been started previously by a different command with a
> postmaster still alive feels like cheating, because pg_ctl is lying
> about its result. If pg_ctl wants to start a cluster but is not able
> to do it, either because the postmaster failed at startup or because
> the cluster has already started, it should report a failure.
I have a same feelings as you. Users may use the return code in their batch file
and they may decide what to do based on the wrong status. Reporting the status
more accurately is nice.
My first idea is that to move the checking part to above, but this may not handle
the case the postmaster is still alive (now sure this is real issue). Do we have to
add a new indicator which ensures the identity of processes for windows?
Please tell me how you feel.
> Now, I
> also recall that the processes spawned by pg_ctl on Windows make the
> status handling rather tricky to reason about..
Did you say about the below comment? Currently I have no idea to make
codes more proper, sorry.
```
* On Windows, we may be checking the postmaster's parent shell, but
* that's fine for this purpose.
```
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
Attachments:
[application/octet-stream] move_checking.patch (1.9K, ../TYAPR01MB58662D7A48E8587A14244097F5EEA@TYAPR01MB5866.jpnprd01.prod.outlook.com/2-move_checking.patch)
download | inline diff:
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index 807d7023a9..07b3f0c9c8 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -599,6 +599,25 @@ wait_for_postmaster_start(pid_t pm_pid, bool do_checkpoint)
char **optlines;
int numlines;
+ /*
+ * Check whether the child postmaster process is still alive. This
+ * lets us exit early if the postmaster fails during startup.
+ *
+ * On Windows, we may be checking the postmaster's parent shell, but
+ * that's fine for this purpose.
+ */
+#ifndef WIN32
+ {
+ int exitstatus;
+
+ if (waitpid(pm_pid, &exitstatus, WNOHANG) == pm_pid)
+ return POSTMASTER_FAILED;
+ }
+#else
+ if (WaitForSingleObject(postmasterProcess, 0) == WAIT_OBJECT_0)
+ return POSTMASTER_FAILED;
+#endif
+
/*
* Try to read the postmaster.pid file. If it's not valid, or if the
* status line isn't there yet, just keep waiting.
@@ -619,6 +638,7 @@ wait_for_postmaster_start(pid_t pm_pid, bool do_checkpoint)
*/
pmpid = atol(optlines[LOCK_FILE_LINE_PID - 1]);
pmstart = atol(optlines[LOCK_FILE_LINE_START_TIME - 1]);
+
if (pmstart >= start_time - 2 &&
#ifndef WIN32
pmpid == pm_pid
@@ -651,25 +671,6 @@ wait_for_postmaster_start(pid_t pm_pid, bool do_checkpoint)
*/
free_readfile(optlines);
- /*
- * Check whether the child postmaster process is still alive. This
- * lets us exit early if the postmaster fails during startup.
- *
- * On Windows, we may be checking the postmaster's parent shell, but
- * that's fine for this purpose.
- */
-#ifndef WIN32
- {
- int exitstatus;
-
- if (waitpid(pm_pid, &exitstatus, WNOHANG) == pm_pid)
- return POSTMASTER_FAILED;
- }
-#else
- if (WaitForSingleObject(postmasterProcess, 0) == WAIT_OBJECT_0)
- return POSTMASTER_FAILED;
-#endif
-
/* Startup still in process; wait, printing a dot once per second */
if (i % WAITS_PER_SEC == 0)
{
view thread (2+ messages)
reply
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: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: RE: pg_ctl start may return 0 even if the postmaster has been already started on Windows
In-Reply-To: <TYAPR01MB58662D7A48E8587A14244097F5EEA@TYAPR01MB5866.jpnprd01.prod.outlook.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox