Date: Sat, 13 Jul 2024 06:41:14 +0900 (JST)
Message-Id: <20240713.064114.901257637518958249.kou@clear-code.com>
To: michael@paquier.xyz
Cc: robertmhaas@gmail.com, horikyota.ntt@gmail.com,
 kuroda.hayato@fujitsu.com, shlok.kyal.oss@gmail.com,
 pgsql-hackers@lists.postgresql.org, tgl@sss.pgh.pa.us
Subject: Re: pg_ctl start may return 0 even if the postmaster has been
 already started on Windows
From: Sutou Kouhei <kou@clear-code.com>
In-Reply-To: <Zl5SC_kboY7i0AS8@paquier.xyz>
References: <20240111.173322.1809044112677090191.horikyota.ntt@gmail.com>
	<CA+TgmoZt44PEeX_=5D7RtpKShPckMyZ9+xQkTnrKU5rnZWjfpQ@mail.gmail.com>
	<Zl5SC_kboY7i0AS8@paquier.xyz>
Mime-Version: 1.0
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/20240713.064114.901257637518958249.kou%40clear-code.com>
Precedence: bulk

Hi,

I'm reviewing patches in Commitfest 2024-07 from top to bottom:
https://commitfest.postgresql.org/48/

This is the 2st patch:
https://commitfest.postgresql.org/48/4573/

FYI: https://commitfest.postgresql.org/48/4681/ is my patch.

In <Zl5SC_kboY7i0AS8@paquier.xyz>
  "Re: pg_ctl start may return 0 even if the postmaster has been already started on Windows" on Tue, 4 Jun 2024 08:30:19 +0900,
  Michael Paquier <michael@paquier.xyz> wrote:

> During a live review of this patch last week, as part of the Advanced
> Patch Workshop of pgconf.dev, it has been mentioned by Tom that we may
> be able to simplify the check on pmstart if the detection of the
> postmaster PID started by pg_ctl is more stable using the WIN32
> internals that this patch relies on.  I am not sure that this
> suggestion is right, though, because we should still care about the
> clock skew case as written in the surrounding comments?  Even if
> that's OK, I would assume that this should be an independent patch,
> written on top of the proposed v6-0001.

I reviewed the latest patch set and I felt different
impression.

start_postmaster() on Windows uses cmd.exe for redirection
based on the comment in the function:

> /*
>  * As with the Unix case, it's easiest to use the shell (CMD.EXE) to
>  * handle redirection etc.  Unfortunately CMD.EXE lacks any equivalent of
>  * "exec", so we don't get to find out the postmaster's PID immediately.
>  */

It seems that we can use redirection by CreateProcess()
family functions without cmd.exe based on the following
documentation:

https://learn.microsoft.com/en-us/windows/win32/procthread/creating-a-child-process-with-redirected-input-and-output

How about changing start_postmaster() for Windows to start
postgres.exe directly so that it returns the postgres.exe's
PID not cmd.exe's PID? If we can do it, we don't need
pgwin32_find_postmaster_pid() in the patch set.


Thanks,
-- 
kou