Re: pg_ctl start may return 0 even if the postmaster has been already started on Windows

Michael Paquier <michael@paquier.xyz>

From: Michael Paquier <michael@paquier.xyz>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>, kuroda.hayato@fujitsu.com, shlok.kyal.oss@gmail.com, pgsql-hackers@lists.postgresql.org
Date: 2024-01-09T00:40:23Z
Lists: pgsql-hackers
On Fri, Jan 05, 2024 at 02:58:55PM -0500, Robert Haas wrote:
> I'm not a Windows expert, but my guess is that 0001 is a very good
> idea. I hope someone who is a Windows expert will comment on that.

I am +1 on 0001.  It is just something we've never anticipated when
these wrappers around cmd in pg_ctl were written.

> 0002 seems problematic to me. One potential issue is that it would
> break if someone renamed postgres.exe to something else -- although
> that's probably not really a serious problem.

We do a find_other_exec_or_die() on "postgres" with what could be a
custom execution path.  So we're actually sure that the binary will be
there in the start path, no?  I don't like much the hardcoded
dependency to .exe here.

> A bigger issue is that
> it seems like it would break if someone used pg_ctl to start several
> instances in different data directories on the same machine. If I'm
> understanding correctly, that currently mostly works, and this would
> break it.

Not having the guarantee that a single shell_pid is associated to a
single postgres.exe would be a problem.  Now the patch includes this
code:
+		if (ppe.th32ParentProcessID == shell_pid &&
+			strcmp("postgres.exe", ppe.szExeFile) == 0)
+		{
+			if (pm_pid != ppe.th32ProcessID && pm_pid != 0)
+				multiple_children = true;
+			pm_pid = ppe.th32ProcessID;
+		}

Which is basically giving this guarantee?  multiple_children should
never happen once the autorun part is removed.  Is that right?

+        * The launcher shell might start other cmd.exe instances or programs
+        * besides postgres.exe. Veryfying the program file name is essential.

With the autorun part of cmd.exe removed, what's still relevant here?
s/Veryfying/Verifying/.

Perhaps 0002 should make more efforts in documenting things like
th32ProcessID and th32ParentProcessID.
--
Michael

Commits

  1. Re-enable autoruns for cmd.exe on Windows

  2. Re-enable autoruns for for cmd.exe on Windows

  3. pg_regress: Disable autoruns for cmd.exe on Windows

  4. pg_ctl: Disable autoruns for cmd.exe on Windows