From: Alexander Lakhin <exclusion@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Justin Pryzby <pryzby@telsasoft.com>
Subject: Re: More time spending with "delete pending"
Date: Thu, 8 Jul 2021 23:00:00 +0300
Message-ID: <d030cc87-641c-c2f0-b7a9-4d6669ea91d7@gmail.com> (raw)
In-Reply-To: <YOatgGjMgnEFsnGL@paquier.xyz>
References: <20201115011112.GW30691@telsasoft.com>
<979f4ee6-9960-e37f-f3ee-f458e87777b0@gmail.com>
<995713.1605725597@sss.pgh.pa.us>
<1001775.1605731995@sss.pgh.pa.us>
<3366bfde-183f-60be-62ab-8ae81b946bac@gmail.com>
<1006364.1605738508@sss.pgh.pa.us>
<20729794-e2bf-bd7a-f01b-d132f89fd0d7@gmail.com>
<fcc8aea0-14d2-9fab-1a4e-37fd1925c49b@gmail.com>
<YOQVX1f9/2s1ct6W@paquier.xyz>
<78fa6a17-8e71-7c99-62bd-8a11a47fe894@gmail.com>
<YOatgGjMgnEFsnGL@paquier.xyz>
08.07.2021 10:47, Michael Paquier wrote:
> On Thu, Jul 08, 2021 at 07:00:01AM +0300, Alexander Lakhin wrote:
>> As Tom Lane noted above, the code added with bed90759f is dubious
>> (_NtQueryInformationFile() can not be used to handle the "delete
>> pending" state as CreateFile() returns INVALID_HANDLE_VALUE in this case.)
>> Probably that change should be reverted. Should I do it along with the
>> proposed fix?
> Ah, I see. I have managed to miss your point. If
> _NtQueryInformationFile() cannot be used, then we'd actually miss the
> contents for standardInfo and the pending deletion. If you could send
> everything you have, that would be helpful! I'd like to test that
> stuff by myself, with all the contents discussed at disposal for a
> proper evaluation.
> --
Beside the aforementioned test I can only propose the extended patch,
that incorporates the undo of the changes brought by bed90759f.
With this patch that test is passed.
Best regards,
Alexander
Attachments:fix_pg_ls_dir_v2.patch (text/x-patch, 6.1K) downloaddiff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.cindex c436d9318b..a8c22a81d0 100644--- a/src/backend/utils/adt/genfile.c+++ b/src/backend/utils/adt/genfile.c@@ -28,6 +28,7 @@
#include "funcapi.h"
#include "mb/pg_wchar.h"
#include "miscadmin.h"
+#include "port.h"
#include "postmaster/syslogger.h"
#include "storage/fd.h"
#include "utils/acl.h"
diff --git a/src/port/win32stat.c b/src/port/win32stat.cindex 2ad8ee1359..6028ad809e 100644--- a/src/port/win32stat.c+++ b/src/port/win32stat.c@@ -18,53 +18,6 @@
#include "c.h"
#include <windows.h>
-/*- * In order to support MinGW and MSVC2013 we use NtQueryInformationFile as an- * alternative for GetFileInformationByHandleEx. It is loaded from the ntdll- * library.- */-#if _WIN32_WINNT < 0x0600-#include <winternl.h>--#if !defined(__MINGW32__) && !defined(__MINGW64__)-/* MinGW includes this in <winternl.h>, but it is missing in MSVC */-typedef struct _FILE_STANDARD_INFORMATION-{- LARGE_INTEGER AllocationSize;- LARGE_INTEGER EndOfFile;- ULONG NumberOfLinks;- BOOLEAN DeletePending;- BOOLEAN Directory;-} FILE_STANDARD_INFORMATION;-#define FileStandardInformation 5-#endif /* !defined(__MINGW32__) &&- * !defined(__MINGW64__) */--typedef NTSTATUS (NTAPI * PFN_NTQUERYINFORMATIONFILE)- (IN HANDLE FileHandle,- OUT PIO_STATUS_BLOCK IoStatusBlock,- OUT PVOID FileInformation,- IN ULONG Length,- IN FILE_INFORMATION_CLASS FileInformationClass);--static PFN_NTQUERYINFORMATIONFILE _NtQueryInformationFile = NULL;--static HMODULE ntdll = NULL;--/*- * Load DLL file just once regardless of how many functions we load/call in it.- */-static void-LoadNtdll(void)-{- if (ntdll != NULL)- return;- ntdll = LoadLibraryEx("ntdll.dll", NULL, 0);-}--#endif /* _WIN32_WINNT < 0x0600 */--
/*
* Convert a FILETIME struct into a 64 bit time_t.
*/
@@ -170,110 +123,73 @@ _pgstat64(const char *name, struct stat *buf)
SECURITY_ATTRIBUTES sa;
HANDLE hFile;
int ret;
-#if _WIN32_WINNT < 0x0600- IO_STATUS_BLOCK ioStatus;- FILE_STANDARD_INFORMATION standardInfo;-#else- FILE_STANDARD_INFO standardInfo;-#endif+ int loops = 0;
if (name == NULL || buf == NULL)
{
errno = EINVAL;
return -1;
}
-
/* fast not-exists check */
if (GetFileAttributes(name) == INVALID_FILE_ATTRIBUTES)
{
- _dosmaperr(GetLastError());- return -1;+ DWORD err = GetLastError();++ if (err != ERROR_ACCESS_DENIED) {+ _dosmaperr(err);+ return -1;+ }
}
/* get a file handle as lightweight as we can */
sa.nLength = sizeof(SECURITY_ATTRIBUTES);
sa.bInheritHandle = TRUE;
sa.lpSecurityDescriptor = NULL;
- hFile = CreateFile(name,- GENERIC_READ,- (FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE),- &sa,- OPEN_EXISTING,- (FILE_FLAG_NO_BUFFERING | FILE_FLAG_BACKUP_SEMANTICS |- FILE_FLAG_OVERLAPPED),- NULL);- if (hFile == INVALID_HANDLE_VALUE)+ while ((hFile = CreateFile(name,+ GENERIC_READ,+ (FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE),+ &sa,+ OPEN_EXISTING,+ (FILE_FLAG_NO_BUFFERING | FILE_FLAG_BACKUP_SEMANTICS |+ FILE_FLAG_OVERLAPPED),+ NULL)) == INVALID_HANDLE_VALUE)
{
DWORD err = GetLastError();
- CloseHandle(hFile);- _dosmaperr(err);- return -1;- }-- memset(&standardInfo, 0, sizeof(standardInfo));--#if _WIN32_WINNT < 0x0600- if (_NtQueryInformationFile == NULL)- {- /* First time through: load ntdll.dll and find NtQueryInformationFile */- LoadNtdll();- if (ntdll == NULL)- {- DWORD err = GetLastError();-- CloseHandle(hFile);- _dosmaperr(err);- return -1;- }-- _NtQueryInformationFile = (PFN_NTQUERYINFORMATIONFILE) (pg_funcptr_t)- GetProcAddress(ntdll, "NtQueryInformationFile");- if (_NtQueryInformationFile == NULL)+ /*+ * ERROR_ACCESS_DENIED is returned if the file is deleted but not yet+ * gone (Windows NT status code is STATUS_DELETE_PENDING). In that+ * case we want to wait a bit and try again, giving up after 1 second+ * (since this condition should never persist very long). However,+ * there are other commonly-hit cases that return ERROR_ACCESS_DENIED,+ * so care is needed. In particular that happens if we try to open a+ * directory, or of course if there's an actual file-permissions+ * problem. To distinguish these cases, try a stat(). In the+ * delete-pending case, it will either also get STATUS_DELETE_PENDING,+ * or it will see the file as gone and fail with ENOENT. In other+ * cases it will usually succeed. The only somewhat-likely case where+ * this coding will uselessly wait is if there's a permissions problem+ * with a containing directory, which we hope will never happen in any+ * performance-critical code paths.+ */+ if (err == ERROR_ACCESS_DENIED)
{
- DWORD err = GetLastError();-- CloseHandle(hFile);- _dosmaperr(err);- return -1;+ if (loops < 10)+ {+ struct microsoft_native_stat st;++ if (microsoft_native_stat(name, &st) != 0)+ {+ pg_usleep(100000);+ loops++;+ continue;+ }+ }
}
- }-- if (!NT_SUCCESS(_NtQueryInformationFile(hFile, &ioStatus, &standardInfo,- sizeof(standardInfo),- FileStandardInformation)))- {- DWORD err = GetLastError();-- CloseHandle(hFile);- _dosmaperr(err);- return -1;- }-#else- if (!GetFileInformationByHandleEx(hFile, FileStandardInfo, &standardInfo,- sizeof(standardInfo)))- {- DWORD err = GetLastError();- CloseHandle(hFile);
_dosmaperr(err);
return -1;
}
-#endif /* _WIN32_WINNT < 0x0600 */-- if (standardInfo.DeletePending)- {- /*- * File has been deleted, but is not gone from the filesystem yet.- * This can happen when some process with FILE_SHARE_DELETE has it- * open, and it will be fully removed once that handle is closed.- * Meanwhile, we can't open it, so indicate that the file just doesn't- * exist.- */- CloseHandle(hFile);- errno = ENOENT;- return -1;- }
/* At last we can invoke fileinfo_to_stat */
ret = fileinfo_to_stat(hFile, buf);
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-hackers@postgresql.org
Cc: exclusion@gmail.com, michael@paquier.xyz, tgl@sss.pgh.pa.us, pryzby@telsasoft.com
Subject: Re: More time spending with "delete pending"
In-Reply-To: <d030cc87-641c-c2f0-b7a9-4d6669ea91d7@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