From: Alexander Lakhin <exclusion@gmail.com>
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
To: tgl@sss.pgh.pa.us
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #16161: pg_ctl stop fails sometimes (on Windows)
Date: Thu, 19 Dec 2019 07:00:00 +0300
Message-ID: <d32a75e6-ba99-c95c-e475-1887c009d395@gmail.com> (raw)
In-Reply-To: <20191219.103948.2182167132387989763.horikyota.ntt@gmail.com>
References: <4741.1576684991@sss.pgh.pa.us>
<b2cc98b1-0ba0-1358-572f-8a1a01a3880e@gmail.com>
<26441.1576706955@sss.pgh.pa.us>
<20191219.103948.2182167132387989763.horikyota.ntt@gmail.com>
19.12.2019 4:39, Kyotaro Horiguchi wrote:
> At Wed, 18 Dec 2019 17:09:15 -0500, Tom Lane <tgl@sss.pgh.pa.us> wrote in
>> Alexander Lakhin <exclusion@gmail.com> writes:
>>> So I see three ways now:
>>> 1) Revert the pgwin32_open change and choose the previously rejected
>>> approach: Unlink postmaster.pid using rename operation (adopt the
>>> solution from https://stackoverflow.com/questions/3764072/).
>>> 2) Add a check for directory in pgwin32_open, but I think then we're
>>> getting close to checking there just for postmaster.pid.
>>> 3) Find out how often we get ERROR_ACCESS_DENIED during all regression
>>> tests and if such error count is near zero, then add the check for isdir
>>> in fsync_fname_ext and friends, and get done.
>> I'm not terribly thrilled with (1) because I do not think that
>> postmaster.pid accesses are the only place where we have this issue.
>> And (3) is not only not a forward-looking solution, but it imagines
>> that the speed of the regression tests is the only thing we need to
>> worry about here. We generally don't assume that the regression
>> tests are an accurate model of the performance users will see.
>>
>> So that leaves us with (2), it seems.
> Agreed for (2).
>
> I'm not sure about the point of the discussion that stat()'ing is
> messy. If it's about performance, does stat()ing only when CreateFile
> returned ERROR_ACCESS_DENIED helps? Anyway we wait for a moment in
> that case.
Please look at the patch that implements (2). It makes `vcregress
recoverycheck` pass (and my demo restart test still passes too).
As open(dir) is getting a little more expensive than before, maybe it's
still worthwhile to patch fsync*(..., isdir,...). I can prepare such a
patch if so.
Best regards,
Alexander
Attachments:
[text/x-patch] improve-eacces-handling.patch (713B, ../d32a75e6-ba99-c95c-e475-1887c009d395@gmail.com/2-improve-eacces-handling.patch)
download | inline diff:diff --git a/src/port/open.c b/src/port/open.cindex 51652762a1..2196cbc992 100644--- a/src/port/open.c+++ b/src/port/open.c@@ -144,11 +144,19 @@ pgwin32_open(const char *fileName, int fileFlags,...)
*/
if (err == ERROR_ACCESS_DENIED)
{
- if (loops < 10)- {- pg_usleep(100000);- loops++;- continue;+ /*+ * We don't want to retry fopen() with a directory,+ * which is known to fail with the same ERROR_ACCESS_DENIED error+ * (Windows NT status code STATUS_FILE_IS_A_DIRECTORY).+ */+ struct stat st;+ if (stat(fileName, &st) != 0 || !S_ISDIR(st.st_mode)) {+ if (loops < 10)+ {+ pg_usleep(100000);+ loops++;+ continue;+ }
}
}
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: pgsql-bugs@postgresql.org
Cc: exclusion@gmail.com, horikyota.ntt@gmail.com, tgl@sss.pgh.pa.us, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #16161: pg_ctl stop fails sometimes (on Windows)
In-Reply-To: <d32a75e6-ba99-c95c-e475-1887c009d395@gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox