[magnus@hagander.net: Re: [HACKERS] pgwin32_open returning EINVAL]

Jaime, si pudieras construir una version parchada para pasarle a Felipe
y que verifique si el problema aún se presenta, te lo agradeceríamos :-)

Creo que el parche puede estropearse al pasar por el correo -- lo puedes
sacar más limpiamente de acá:
http://thread.gmane.org/gmane.comp.db.postgresql.devel.general/89841/focus=90723
al menos lo pude aplicar limpiamente sin problemas copiando/pegando
desde gmane.

Gracias!

(FWIW yo creo que sería bueno agregarle lo que sugiere Magnus: si
después de, digamos, 5 intentos sigue en el loop, mandar un WARNING para
que el usuario tenga indicación de lo que está pasando)

----- Forwarded message from Magnus Hagander <magnus(at)hagander(dot)net> -----

From: Magnus Hagander <magnus(at)hagander(dot)net>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Gregory Stark <stark(at)enterprisedb(dot)com>,
Andrew Dunstan <andrew(at)dunslane(dot)net>,
Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>,
Martijn van Oosterhout <kleptog(at)svana(dot)org>,
Hackers <pgsql-hackers(at)postgresql(dot)org>
Date: Wed, 19 Dec 2007 13:52:32 +0100
Subject: Re: [HACKERS] pgwin32_open returning EINVAL
Message-ID: <20071219125232(dot)GL11226(at)svr2(dot)hagander(dot)net>

On Sat, Dec 15, 2007 at 10:31:38PM -0500, Tom Lane wrote:
> Gregory Stark <stark(at)enterprisedb(dot)com> writes:
> > "Andrew Dunstan" <andrew(at)dunslane(dot)net> writes:
> >> Interesting. Maybe forever is going a bit too far, but retrying for <n>
> >> seconds or so.
>
> > I think looping forever is the right thing. Having a fixed timeout just means
> > Postgres will break sometimes instead of all the time. And it introduces
> > non-deterministic behaviour too.
>
> Looping forever would be considered broken by a very large fraction of
> the community.
>
> IIRC we have a 30-second timeout in rename() for Windows, and that seems
> to be working well enough, so I'd be inclined to copy the behavior for
> this case.

Here's a patch that I think implements this ;) Alvaro - do you have a build
env so you can test it? I can't reproduce the problem in my environment...

Also, it currently just silently loops. Would it be interesting to
ereport(WARNING) that it's looping on the open, to let the user know
there's a problem? (Naturally, only warning the first time it tries it on
each file, so we don't spam the log too hard)

//Magnus

Index: src/port/open.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/port/open.c,v
retrieving revision 1.22
diff -c -r1.22 open.c
*** src/port/open.c 30 Nov 2007 11:16:43 -0000 1.22
--- src/port/open.c 19 Dec 2007 12:42:05 -0000
***************
*** 58,65 ****
pgwin32_open(const char *fileName, int fileFlags,...)
{
int fd;
! HANDLE h;
SECURITY_ATTRIBUTES sa;

/* Check that we can handle the request */
assert((fileFlags & ((O_RDONLY | O_WRONLY | O_RDWR) | O_APPEND |
--- 58,66 ----
pgwin32_open(const char *fileName, int fileFlags,...)
{
int fd;
! HANDLE h = INVALID_HANDLE_VALUE;
SECURITY_ATTRIBUTES sa;
+ int loops = 0;

/* Check that we can handle the request */
assert((fileFlags & ((O_RDONLY | O_WRONLY | O_RDWR) | O_APPEND |
***************
*** 71,77 ****
sa.bInheritHandle = TRUE;
sa.lpSecurityDescriptor = NULL;

! if ((h = CreateFile(fileName,
/* cannot use O_RDONLY, as it == 0 */
(fileFlags & O_RDWR) ? (GENERIC_WRITE | GENERIC_READ) :
((fileFlags & O_WRONLY) ? GENERIC_WRITE : GENERIC_READ),
--- 72,78 ----
sa.bInheritHandle = TRUE;
sa.lpSecurityDescriptor = NULL;

! while ((h = CreateFile(fileName,
/* cannot use O_RDONLY, as it == 0 */
(fileFlags & O_RDWR) ? (GENERIC_WRITE | GENERIC_READ) :
((fileFlags & O_WRONLY) ? GENERIC_WRITE : GENERIC_READ),
***************
*** 88,93 ****
--- 89,108 ----
((fileFlags & O_DSYNC) ? FILE_FLAG_WRITE_THROUGH : 0),
NULL)) == INVALID_HANDLE_VALUE)
{
+ /*
+ * Sharing violation or locking error can indicate antivirus, backup
+ * or similar software that's locking the file. Try again for 30 seconds
+ * before giving up.
+ */
+ if (GetLastError() == ERROR_SHARING_VIOLATION ||
+ GetLastError() == ERROR_LOCK_VIOLATION)
+ {
+ pg_usleep(100000);
+ loops++;
+ if (loops < 30)
+ continue;
+ }
+
_dosmaperr(GetLastError());
return -1;
}

----- End forwarded message -----

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.