Re: BUG #16161: pg_ctl stop fails sometimes (on Windows)
Alexander Lakhin <[email protected]>
| Newsgroups | gmane.comp.db.postgresql.bugs |
|---|---|
| Message-ID | <[email protected]> |
11.12.2019 23:00, PG Bug reporting form wrote: > The following bug has been logged on the website: > > Bug reference: 16161 > Logged by: Alexander Lakhin > Email address: [email protected] > PostgreSQL version: 12.1 > Operating system: Windows > Description: > > The regression tests on Windows sometimes fail with 'Permission denied' > errors. For example: > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=dory&dt=2019-12-11%2007%3A45%3A33 > > ============== shutting down postmaster ============== > pg_ctl: could not open PID file > "c:/pgbuildfarm/pgbuildroot/HEAD/pgsql.build/src/test/regress/./tmp_check/data/postmaster.pid": > Permission denied > > This error occurs when pg_ctl is trying to open postmaster.pid while this > file is in "delete pending" state > (https://stackoverflow.com/questions/3764072/). To reproduce the issue reliably I propose a simple modification to synchronize unlink() with open() and a simple test (sync_pid_ops+test.patch). With the patch applied, `vcregress taptest src/test/restart` fails for me on iteration 47, 6, 7, 42, 51, 26, ... I see two ways to fix the issue: 1) Unlink postmaster.pid using rename operation (adopt the solution from https://stackoverflow.com/questions/3764072/). 2) Ignore such 'Permission denied' error and just try to open the file once again (attached fix_open_for_unlink.patch implements this). I'm inclined to the second approach as pgwin32_open() already handles two transient states (Windows-only), and it could be useful not only for postmaster.pid, but for some other files. Best regards, Alexander
sync_pid_ops+test.patch
(text/x-patch, 1.6 KB)
diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c
index de554e28cf..0594b6649f 100644
--- a/src/backend/utils/init/miscinit.c
+++ b/src/backend/utils/init/miscinit.c
@@ -838,6 +838,13 @@ UnlinkLockFiles(int status, Datum arg)
{
ListCell *l;
+#ifdef WIN32
+{
+FILETIME time;
+do { GetSystemTimePreciseAsFileTime(&time); }
+while (((((ULONGLONG) time.dwHighDateTime) << 32) + time.dwLowDateTime) % 10000000L > (10000L));
+}
+#endif
foreach(l, lock_files)
{
char *curfile = (char *) lfirst(l);
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index b0858411ed..3ab1608a5d 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -276,6 +276,14 @@ get_pgpid(bool is_status_request)
exit(is_status_request ? 4 : 1);
}
+#ifdef WIN32
+{
+FILETIME time;
+do { GetSystemTimePreciseAsFileTime(&time); }
+while (((((ULONGLONG) time.dwHighDateTime) << 32) + time.dwLowDateTime) % 10000000L > (10000L));
+}
+#endif
+
pidf = fopen(pid_file, "r");
if (pidf == NULL)
{
diff --git a/src/test/restart/t/001_restart.pl b/src/test/restart/t/001_restart.pl
new file mode 100644
index 0000000000..f0621d7f27
--- /dev/null
+++ b/src/test/restart/t/001_restart.pl
@@ -0,0 +1,20 @@
+use strict;
+use warnings;
+use PostgresNode;
+use TestLib;
+use Test::More;
+use Config;
+
+my $node = get_new_node('master');
+$node->init();
+
+for ( my $i = 1; $i <= 1000; $i++ ) {
+ my ($sec,$min,$hour,$mday,$mon,$year,$wday,$yday,$isdst) = localtime();
+ diag( sprintf("%02d:%02d:%02d iteration %d", $hour, $min, $sec, $i) );
+ $node->start;
+ $node->stop;
+}
+
+ok(1, 'node start/stop test');
+
+done_testing();
fix_open_for_unlink.patch
(text/x-patch, 668 B)
diff --git a/src/port/open.c b/src/port/open.c
index f37afc7512..dad328f000 100644
--- a/src/port/open.c
+++ b/src/port/open.c
@@ -135,6 +135,21 @@ pgwin32_open(const char *fileName, int fileFlags,...)
continue;
}
+ /*
+ * ERROR_ACCESS_DENIED can be returned while the file is deleted
+ * (Windows NT status code is STATUS_DELETE_PENDING).
+ * https://stackoverflow.com/questions/3764072/
+ * Try again for 1 second to check whether the "access denied"
+ * condition persists.
+ */
+ if (err == ERROR_ACCESS_DENIED) {
+ if (loops < 10) {
+ pg_usleep(100000);
+ loops++;
+ continue;
+ }
+ }
+
_dosmaperr(err);
return -1;
}