Horton

Re: BUG #16161: pg_ctl stop fails sometimes (on Windows)

Tom Lane <tgl@sss.pgh.pa.us>

View thread View on postgr.es
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Alexander Lakhin <exclusion@gmail.com>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>, pgsql-bugs@lists.postgresql.org
Date: 2019-12-19T15:44:08Z
Lists: pgsql-bugs

Attachments

Alexander Lakhin <exclusion@gmail.com> writes:
> Please look at the patch that implements (2). It makes `vcregress
> recoverycheck` pass (and my demo restart test still passes too).

I think we could be a little smarter here.  If the problem is
STATUS_DELETE_PENDING, doesn't that affect stat() as well?  That is,
if stat() succeeds, we needn't wait, independently of whether it
says S_ISDIR or not?  This seems like a noticeable improvement if
true, because it would mean that ordinary file-permission failures
also need not wait.  We'd still get confused if we get a permission
failure on a containing directory rather than the file itself, but
that I'm prepared to dismiss as an uncommon case.

Entirely-untested patch along this line attached.

BTW ... it's likely that stat() here is actually going to invoke
pgwin32_safestat(), which has its own opinions about this, and
indeed seems to think we'll get ERROR_DELETE_PENDING.  I think
that's harmless here, but it makes me wonder if we should use
a native Windows API instead of going through stat().

> 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.

I think we should leave that for later, so that the buildfarm can
actually test whatever logic we put in here.

			regards, tom lane

Commits

  1. In pgwin32_open, loop after ERROR_ACCESS_DENIED only if we can't stat.

  2. On Windows, wait a little to see if ERROR_ACCESS_DENIED goes away.