Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1iheYK-0002iU-Rr for pgsql-bugs@arkaria.postgresql.org; Wed, 18 Dec 2019 19:00:09 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1iheYJ-0004Hi-Ka for pgsql-bugs@arkaria.postgresql.org; Wed, 18 Dec 2019 19:00:07 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1iheYJ-0004Hb-92 for pgsql-bugs@lists.postgresql.org; Wed, 18 Dec 2019 19:00:07 +0000 Received: from mail-lj1-x242.google.com ([2a00:1450:4864:20::242]) by magus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1iheYF-000363-NB for pgsql-bugs@lists.postgresql.org; Wed, 18 Dec 2019 19:00:06 +0000 Received: by mail-lj1-x242.google.com with SMTP id l2so3339103lja.6 for ; Wed, 18 Dec 2019 11:00:03 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language; bh=SET5ArJk9Z9tqUiM7Sibj4RRr2AkTvUopOqq9Z3MJBo=; b=qlARBlAJRVD1Qdn4rMeF9umA/9FeuGPH17uewa2uNsYzU/iXXfU9HSGwR1bihDFzS6 MFg0uLPB9aHnq8g8gpEfH6Ygfj55e2yoFpYXoIVWkrxhs4bLZ6LHw/yzhD6x546F2CAe q6aaE1sQfPH/wehYEZ/jU0eYiOC1kXkgKo6jr7nOl0FLEvXmzcwKKWMceIKdKtQ2kS5R BLAcsoFCEJ8THW0G7aH1QmRJPSC0btPp1IxMjdKZ3CRCOSsmdHbXcchcYtbeE1oRqA8i Itj3vETNI21GUy2kf8Gcdv+UuXaWzPDSNOu5ld2KofiOjnbGz8+e9nqfSbCS0sEUvHNV 5vsg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language; bh=SET5ArJk9Z9tqUiM7Sibj4RRr2AkTvUopOqq9Z3MJBo=; b=VfJ8O5GFGyOz83WFLulYMeRvfcPKbpOYf0cuoXJtsx9/2IbokN+g2O/kyruD/Ymyab Nb+ohOII0dqi19oGnv+3xVIArVfyMLmSA2riyINvCmu3cui+XFxyVyskCxVyKGt3drCi AUI2JNQSyuV5934EPvzYwoT0+NBW5TXIrf/DhH9FXslMHgQtA0AcPQvn29NlQDcr3LyW iAJ3B6Oo6ub3z5EyQl69kCE9JUIRFy7x5UjRyhnP9fBg7B05vD+KwqSTMKxlAi1Mn2zn swKWi6zk3LEyoSXCpLjX9NliILIXEVu38sMczGW2iFaKwsGHz+aliQgfbQMbKywKhiFj QmTw== X-Gm-Message-State: APjAAAWWOwDxUlEepIQuYe+ZQrxKaoAn2zaQvaehWL1V1f3GbD9IL+6z LhRXFvbBSpv2dFpJIxXtt3qHE5P6 X-Google-Smtp-Source: APXvYqyyHpJH6TQQpVlmsi5lCyS8FOpPy89KdIc8WmXpTJtguFNINdf1h4e4i+STamtFJW6KFwlVHw== X-Received: by 2002:a2e:2283:: with SMTP id i125mr2962059lji.244.1576695603034; Wed, 18 Dec 2019 11:00:03 -0800 (PST) Received: from [1.0.0.7] ([178.155.4.219]) by smtp.gmail.com with ESMTPSA id n30sm2005811lfi.54.2019.12.18.11.00.02 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 18 Dec 2019 11:00:02 -0800 (PST) Subject: Re: BUG #16161: pg_ctl stop fails sometimes (on Windows) To: Tom Lane Cc: Kyotaro Horiguchi , pgsql-bugs@lists.postgresql.org References: <18124.1576527433@sss.pgh.pa.us> <58a715ac-d118-e7c1-733a-46fa252b345b@gmail.com> <23073.1576626626@sss.pgh.pa.us> <20191218.122526.233778911708036076.horikyota.ntt@gmail.com> <4741.1576684991@sss.pgh.pa.us> From: Alexander Lakhin Message-ID: Date: Wed, 18 Dec 2019 22:00:01 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.2 MIME-Version: 1.0 In-Reply-To: <4741.1576684991@sss.pgh.pa.us> Content-Type: multipart/alternative; boundary="------------F76B1B18079E054C9797C8F5" Content-Language: en-US List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk This is a multi-part message in MIME format. --------------F76B1B18079E054C9797C8F5 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit 18.12.2019 19:03, Tom Lane wrote: > Alexander Lakhin writes: >> 18.12.2019 6:25, Kyotaro Horiguchi wrote: >>> At Tue, 17 Dec 2019 18:50:26 -0500, Tom Lane wrote in >>>> It does look suspiciously like this wait is triggering on these >>>> machines and somehow getting masked in most other test cases. >> https://web.archive.org/web/20150317121919/http://support.microsoft.com/en-us/kb/113996 >> tells that STATUS_FILE_IS_A_DIRECTORY is mapped to ERROR_ACCESS_DENIED too. > Ugh. I wondered if we could really get away with slowing down all > ERROR_ACCESS_DENIED cases. > >> It seems that the cause of the issue is in fsync_fname_ext: > I think you're blaming the messenger. We cannot suddenly impose a > rule that people mustn't try to open() files that might be directories. > Even if fsync_fname_ext() happens to be the only place that does that > today (which I doubt) it's going to bite us on the rear regularly in > the future, because there's no way to enforce it, and most developers > aren't going to notice the problem in testing. Regarding fsync_fname_ext(), I thought that it's OK to avoid performing a call that is known to fail. And even if someone will try to open() a directory, he will need to add the same check for EACCES, otherwise the code will fail on Windows. In fact, I see the same pattern in pre_sync_fname, fsync_fname in file_utils.c. > It's possible that we could deal with this specific case by having > pgwin32_open() try to stat the file and see if it's a directory. But > I think that's messy. Also, the list of kernel-level conditions that > that webpage says are mapped to ERROR_ACCESS_DENIED is scarily long: > > STATUS_THREAD_IS_TERMINATING ERROR_ACCESS_DENIED > STATUS_PROCESS_IS_TERMINATING ERROR_ACCESS_DENIED > STATUS_INVALID_LOCK_SEQUENCE ERROR_ACCESS_DENIED > STATUS_INVALID_VIEW_SIZE ERROR_ACCESS_DENIED > STATUS_ALREADY_COMMITTED ERROR_ACCESS_DENIED > STATUS_ACCESS_DENIED ERROR_ACCESS_DENIED > STATUS_FILE_IS_A_DIRECTORY ERROR_ACCESS_DENIED > STATUS_CANNOT_DELETE ERROR_ACCESS_DENIED > STATUS_FILE_DELETED ERROR_ACCESS_DENIED > STATUS_FILE_RENAMED ERROR_ACCESS_DENIED > STATUS_DELETE_PENDING ERROR_ACCESS_DENIED > STATUS_PORT_CONNECTION_REFUSED ERROR_ACCESS_DENIED > ... > STATUS_ENCRYPTION_FAILED ERROR_ACCESS_DENIED > STATUS_DECRYPTION_FAILED ERROR_ACCESS_DENIED > STATUS_NO_RECOVERY_POLICY ERROR_ACCESS_DENIED > STATUS_NO_EFS ERROR_ACCESS_DENIED > STATUS_WRONG_EFS ERROR_ACCESS_DENIED > STATUS_NO_USER_KEYS ERROR_ACCESS_DENIED > ... > SEC_E_MESSAGE_ALTERED ERROR_ACCESS_DENIED > SEC_E_OUT_OF_SEQUENCE ERROR_ACCESS_DENIED > > Maybe most of these can't occur during fopen, but I'd > rather not assume that I see your point about future uncertainty, but I don't think that receiving e.g. STATUS_FILE_DELETED or STATUS_NO_EFS error every few seconds and ignoring it is a normal practice. > Is there a way to get the original kernel error code? > It'd be a lot better if we could be sure that the condition > is STATUS_DELETE_PENDING before looping. Unfortunately no, there is no known way  to get the kernel error code. As stated in https://stackoverflow.com/questions/3764072, there is the GetFileInformationByHandleEx function, that can return DeletePending state, but to use it we should open the file first (to get a file handle). 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 still thinking that we shouldn't perform doomed calls, but I'm ready to change the course. Best regards, Alexander --------------F76B1B18079E054C9797C8F5 Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: 8bit
18.12.2019 19:03, Tom Lane wrote:
Alexander Lakhin <exclusion@gmail.com> writes:
18.12.2019 6:25, Kyotaro Horiguchi wrote:
At Tue, 17 Dec 2019 18:50:26 -0500, Tom Lane <tgl@sss.pgh.pa.us> wrote in 
It does look suspiciously like this wait is triggering on these
machines and somehow getting masked in most other test cases.

      
https://web.archive.org/web/20150317121919/http://support.microsoft.com/en-us/kb/113996
tells that STATUS_FILE_IS_A_DIRECTORY is mapped to ERROR_ACCESS_DENIED too.
Ugh.  I wondered if we could really get away with slowing down all
ERROR_ACCESS_DENIED cases.

It seems that the cause of the issue is in fsync_fname_ext:
I think you're blaming the messenger.  We cannot suddenly impose a
rule that people mustn't try to open() files that might be directories.
Even if fsync_fname_ext() happens to be the only place that does that
today (which I doubt) it's going to bite us on the rear regularly in
the future, because there's no way to enforce it, and most developers
aren't going to notice the problem in testing.
Regarding fsync_fname_ext(), I thought that it's OK to avoid performing a call that is known to fail. And even if someone will try to open() a directory, he will need to add the same check for EACCES, otherwise the code will fail on Windows. In fact, I see the same pattern in pre_sync_fname, fsync_fname in file_utils.c.
It's possible that we could deal with this specific case by having
pgwin32_open() try to stat the file and see if it's a directory.  But
I think that's messy.  Also, the list of kernel-level conditions that
that webpage says are mapped to ERROR_ACCESS_DENIED is scarily long:

STATUS_THREAD_IS_TERMINATING            ERROR_ACCESS_DENIED
STATUS_PROCESS_IS_TERMINATING           ERROR_ACCESS_DENIED
STATUS_INVALID_LOCK_SEQUENCE            ERROR_ACCESS_DENIED
STATUS_INVALID_VIEW_SIZE                ERROR_ACCESS_DENIED
STATUS_ALREADY_COMMITTED                ERROR_ACCESS_DENIED
STATUS_ACCESS_DENIED                    ERROR_ACCESS_DENIED
STATUS_FILE_IS_A_DIRECTORY              ERROR_ACCESS_DENIED
STATUS_CANNOT_DELETE                    ERROR_ACCESS_DENIED
STATUS_FILE_DELETED                     ERROR_ACCESS_DENIED
STATUS_FILE_RENAMED                     ERROR_ACCESS_DENIED
STATUS_DELETE_PENDING                   ERROR_ACCESS_DENIED
STATUS_PORT_CONNECTION_REFUSED          ERROR_ACCESS_DENIED
...
STATUS_ENCRYPTION_FAILED                ERROR_ACCESS_DENIED
STATUS_DECRYPTION_FAILED                ERROR_ACCESS_DENIED
STATUS_NO_RECOVERY_POLICY               ERROR_ACCESS_DENIED
STATUS_NO_EFS                           ERROR_ACCESS_DENIED
STATUS_WRONG_EFS                        ERROR_ACCESS_DENIED
STATUS_NO_USER_KEYS                     ERROR_ACCESS_DENIED
...
SEC_E_MESSAGE_ALTERED                   ERROR_ACCESS_DENIED   
SEC_E_OUT_OF_SEQUENCE                   ERROR_ACCESS_DENIED   

Maybe most of these can't occur during fopen, but I'd
rather not assume that
I see your point about future uncertainty, but I don't think that receiving e.g. STATUS_FILE_DELETED or STATUS_NO_EFS error every few seconds and ignoring it is a normal practice.
Is there a way to get the original kernel error code?
It'd be a lot better if we could be sure that the condition
is STATUS_DELETE_PENDING before looping.
Unfortunately no, there is no known way  to get the kernel error code. As stated in https://stackoverflow.com/questions/3764072, there is the GetFileInformationByHandleEx function, that can return DeletePending state, but to use it we should open the file first (to get a file handle).

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 still thinking that we shouldn't perform doomed calls, but I'm ready to change the course.

Best regards,
Alexander
--------------F76B1B18079E054C9797C8F5--