Thread: Re: [ADMIN] ERROR: could not read block

I come up with a patch to fix server-side problem. The basic idea is to
convert ERROR_NO_SYSTEM_RESOURCES to EINTR and add code to do retry unless
a new error encountered or successfully done. I tweak the FileRead() logic
on "returnCode <= 0" a little bit by separating it to "<0" and "==0"
parts. This is because if our read passed EOF, read() will not set errno
which may cause a dead loop if a previous read() is interrupted.

For windows, I set a one second waiting time - this should be ok since the
problem is very rare. If the error is permenate, you can always SIGINT the
process since the waiting is done by pg_usleep().

Regards,
Qingqing

---

Index: src/backend/storage/file/fd.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/storage/file/fd.c,v
retrieving revision 1.122
diff -c -r1.122 fd.c
*** src/backend/storage/file/fd.c    22 Nov 2005 18:17:20 -0000    1.122
--- src/backend/storage/file/fd.c    1 Dec 2005 01:09:59 -0000
***************
*** 1009,1019 ****     if (returnCode < 0)         return returnCode;

!     returnCode = read(VfdCache[file].fd, buffer, amount);
!     if (returnCode > 0)
!         VfdCache[file].seekPos += returnCode;
!     else
!         VfdCache[file].seekPos = FileUnknownPos;
     return returnCode; }
--- 1009,1052 ----     if (returnCode < 0)         return returnCode;

!     for (;;)
!     {
!         returnCode = read(VfdCache[file].fd, buffer, amount);
!
!         if (returnCode > 0)
!             VfdCache[file].seekPos += returnCode;
!         else if (returnCode == 0)
!             VfdCache[file].seekPos = FileUnknownPos;
!         else
!         {
! #ifdef WIN32
!             DWORD error = GetLastError();
!
!             switch (error)
!             {
!                 /*
!                  * Since we are using buffered IO now, so windows may run
!                  * out of kernel buffer and return a "Insufficient system
!                  * resources" error. Retry to solve it.
!                  */
!                 case ERROR_NO_SYSTEM_RESOURCES:
!                     pg_usleep(1000);
!                     errno = EINTR;
!                     break;
!                 default:
!                     _dosmaperr(error);
!                     Assert(errno != EINTR);
!             }
! #endif
!             /* Ok if interrupted and retry */
!             if (errno == EINTR)
!                 continue;
!
!             VfdCache[file].seekPos = FileUnknownPos;
!         }
!
!         break;
!     }
     return returnCode; }
***************
*** 1033,1049 ****     if (returnCode < 0)         return returnCode;

!     errno = 0;
!     returnCode = write(VfdCache[file].fd, buffer, amount);

!     /* if write didn't set errno, assume problem is no disk space */
!     if (returnCode != amount && errno == 0)
!         errno = ENOSPC;

!     if (returnCode > 0)
!         VfdCache[file].seekPos += returnCode;
!     else
!         VfdCache[file].seekPos = FileUnknownPos;
     return returnCode; }
--- 1066,1108 ----     if (returnCode < 0)         return returnCode;

!     for (;;)
!     {
!         errno = 0;
!         returnCode = write(VfdCache[file].fd, buffer, amount);

!         /* if write didn't set errno, assume problem is no disk space */
!         if (returnCode != amount && errno == 0)
!             errno = ENOSPC;

!         if (returnCode > 0)
!             VfdCache[file].seekPos += returnCode;
!         else
!         {
! #ifdef WIN32
!             DWORD error = GetLastError();
!
!             switch (error)
!             {
!                 /* see comments in FileRead() */
!                 case ERROR_NO_SYSTEM_RESOURCES:
!                     pg_usleep(1000);
!                     errno = EINTR;
!                     break;
!                 default:
!                     _dosmaperr(error);
!                     Assert(errno != EINTR);
!             }
! #endif
!             /* Ok if interrupted and retry */
!             if (errno == EINTR)
!                 continue;
!
!             VfdCache[file].seekPos = FileUnknownPos;
!         }
!
!         break;
!     }
     return returnCode; }

Qingqing Zhou <zhouqq@cs.toronto.edu> writes:
> !                 default:
> !                     _dosmaperr(error);
> !                     Assert(errno != EINTR);

What's the point of that ... didn't it already happen inside read()?
        regards, tom lane

On Thu, 1 Dec 2005, Tom Lane wrote:

> Qingqing Zhou <zhouqq@cs.toronto.edu> writes:
> > !                 default:
> > !                     _dosmaperr(error);
> > !                     Assert(errno != EINTR);
>
> What's the point of that ... didn't it already happen inside read()?
>

Recall that we have some reports that read() failed to convert some
windows error number to some meaningful errno. For example, the
ERROR_SHARING_VIOLATION error was converted to EINVAL. So we do it
ourselves here and we can get better diagnostic information if this error
is reported again.

Regards,
Qingqing

Qingqing Zhou <zhouqq@cs.toronto.edu> writes:
> I come up with a patch to fix server-side problem.

Applied.

> For windows, I set a one second waiting time -

The code actually does one millisecond; I left it that way since it
seems a reasonable value.
        regards, tom lane