Bruce Momjian wrote:
> Tom Lane wrote:
> > Bruce Momjian <[EMAIL PROTECTED]> writes:
> > > In this way, no one ever has the rename file open while we are holding
> > > the locks, and we can loop without holding an exclusive lock on
> > > pg_shadow, and file writes remain in order.
> > 
> > You're doing this where exactly, and are certain that you are holding no
> > locks why exactly?  And if you aren't holding a lock, what prevents
> > concurrency bugs?
> 
> Oh, for concurrency bugs, you make realfile.new while holding the
> exclusive lock, so someone could come in later and replace realfile.new
> while I am in the rename loop, but then I just install theirs instead.
> 
> I could install someone who has just done the rename to realfile.new but
> not tried the rename from realfile.new to realfile, but that seems OK. 
> They will just fine the file missing and fail on the rename, which is
> OK.

OK, here is a patch that I think handles rename.  It does the rename to
a secondary file while holding the lock, then releases the lock and does
a rename to the active file.  I enabled this for Win32 and Cygwin, which
has the same file system behavior.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  [EMAIL PROTECTED]               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
Index: src/backend/commands/user.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/commands/user.c,v
retrieving revision 1.133
diff -c -c -r1.133 user.c
*** src/backend/commands/user.c 26 Jan 2004 22:35:32 -0000      1.133
--- src/backend/commands/user.c 27 Jan 2004 03:46:00 -0000
***************
*** 139,145 ****
        bufsize = strlen(filename) + 12;
        tempname = (char *) palloc(bufsize);
        snprintf(tempname, bufsize, "%s.%d", filename, MyProcPid);
! 
        oumask = umask((mode_t) 077);
        fp = AllocateFile(tempname, "w");
        umask(oumask);
--- 139,149 ----
        bufsize = strlen(filename) + 12;
        tempname = (char *) palloc(bufsize);
        snprintf(tempname, bufsize, "%s.%d", filename, MyProcPid);
! #if defined(WIN32) || defined(CYGWIN)
!       filename = repalloc(filename, strlen(filename) + 1 + strlen(".new");
!       strcat(filename, ".new");
! #endif
!       
        oumask = umask((mode_t) 077);
        fp = AllocateFile(tempname, "w");
        umask(oumask);
***************
*** 286,291 ****
--- 290,299 ----
        bufsize = strlen(filename) + 12;
        tempname = (char *) palloc(bufsize);
        snprintf(tempname, bufsize, "%s.%d", filename, MyProcPid);
+ #if defined(WIN32) || defined(CYGWIN)
+       filename = repalloc(filename, strlen(filename) + 1 + strlen(".new");
+       strcat(filename, ".new");
+ #endif
  
        oumask = umask((mode_t) 077);
        fp = AllocateFile(tempname, "w");
***************
*** 457,462 ****
--- 465,482 ----
                user_file_update_needed = false;
                write_user_file(urel);
                heap_close(urel, NoLock);
+ #if defined(WIN32) || defined(CYGWIN)
+               {
+                       /* Rename active file while not holding an exclusive lock */
+                       char *filename = user_getfilename(), *filename_new;
+ 
+                       filename_new = palloc(strlen(filename) + 1 + strlen(".new")));
+                       sprintf(filename_new, "%s.new", filename);
+                       rename(filename_new, filename);
+                       pfree(filename);
+                       pfree(filename_new);
+               }
+ #endif
        }
  
        if (group_file_update_needed)
***************
*** 464,469 ****
--- 484,501 ----
                group_file_update_needed = false;
                write_group_file(grel);
                heap_close(grel, NoLock);
+ #if defined(WIN32) || defined(CYGWIN)
+               {
+                       /* Rename active file while not holding an exclusive lock */
+                       char *filename = group_getfilename(), *filename_new;
+ 
+                       filename_new = palloc(strlen(filename) + 1 + strlen(".new")));
+                       sprintf(filename_new, "%s.new", filename);
+                       rename(filename_new, filename);
+                       pfree(filename);
+                       pfree(filename_new);
+               }
+ #endif
        }
  
        /*
Index: src/backend/utils/cache/relcache.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/utils/cache/relcache.c,v
retrieving revision 1.195
diff -c -c -r1.195 relcache.c
*** src/backend/utils/cache/relcache.c  26 Jan 2004 22:35:32 -0000      1.195
--- src/backend/utils/cache/relcache.c  27 Jan 2004 03:46:04 -0000
***************
*** 3358,3390 ****
                /*
                 * OK, rename the temp file to its final name, deleting any
                 * previously-existing init file.
-                *
-                * Note: a failure here is possible under Cygwin, if some other
-                * backend is holding open an unlinked-but-not-yet-gone init file.
-                * So treat this as a noncritical failure.
                 */
!               if (rename(tempfilename, finalfilename) < 0)
                {
!                       ereport(WARNING,
!                                       (errcode_for_file_access(),
!                               errmsg("could not rename relation-cache initialization 
file \"%s\" to \"%s\": %m",
!                                          tempfilename, finalfilename),
!                                        errdetail("Continuing anyway, but there's 
something wrong.")));
! 
!                       /*
!                        * If we fail, try to clean up the useless temp file; don't
!                        * bother to complain if this fails too.
!                        */
!                       unlink(tempfilename);
                }
        }
        else
        {
                /* Delete the already-obsolete temp file */
                unlink(tempfilename);
        }
- 
-       LWLockRelease(RelCacheInitLock);
  }
  
  /*
--- 3358,3385 ----
                /*
                 * OK, rename the temp file to its final name, deleting any
                 * previously-existing init file.
                 */
! #if defined(WIN32) || defined(CYGWIN)
!               rename(tempfilename, finalfilename);
!               LWLockRelease(RelCacheInitLock);
! #else
                {
!                       char            finalfilename_new[MAXPGPATH];
!       
!                       snprintf(finalfilename_new, sizeof(finalfilename_new), 
"%s.new", finalfilename);
!                       rename(tempfilename, finalfilename_new);
!                       LWLockRelease(RelCacheInitLock);
!                       /* Rename to active file after lock is released */
!                       rename(finalfilename_new, finalfilename);
                }
+ #endif
        }
        else
        {
                /* Delete the already-obsolete temp file */
                unlink(tempfilename);
+               LWLockRelease(RelCacheInitLock);
        }
  }
  
  /*
Index: src/backend/utils/misc/guc.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/utils/misc/guc.c,v
retrieving revision 1.181
diff -c -c -r1.181 guc.c
*** src/backend/utils/misc/guc.c        26 Jan 2004 22:35:32 -0000      1.181
--- src/backend/utils/misc/guc.c        27 Jan 2004 03:46:07 -0000
***************
*** 3981,3987 ****
                return;
        }
  
!       /* Put new file in place, this could delay on Win32 */
        rename(new_filename, filename);
        free(new_filename);
        free(filename);
--- 3981,3990 ----
                return;
        }
  
!       /*
!        *      Put new file in place.  This could delay on Win32, but we don't hold
!        *      any exclusive locks.
!        */
        rename(new_filename, filename);
        free(new_filename);
        free(filename);
Index: src/include/port.h
===================================================================
RCS file: /cvsroot/pgsql-server/src/include/port.h,v
retrieving revision 1.15
diff -c -c -r1.15 port.h
*** src/include/port.h  29 Nov 2003 22:40:53 -0000      1.15
--- src/include/port.h  27 Jan 2004 03:46:07 -0000
***************
*** 30,40 ****
  extern off_t ftello(FILE *stream);
  #endif
  
! #ifdef WIN32
  /*
   * Win32 doesn't have reliable rename/unlink during concurrent access
   */
- #ifndef FRONTEND
  extern int    pgrename(const char *from, const char *to);
  extern int    pgunlink(const char *path);
  
--- 30,39 ----
  extern off_t ftello(FILE *stream);
  #endif
  
! #if !defined(FRONTEND) && (defined(WIN32) || defined(CYGWIN))
  /*
   * Win32 doesn't have reliable rename/unlink during concurrent access
   */
  extern int    pgrename(const char *from, const char *to);
  extern int    pgunlink(const char *path);
  
***************
*** 42,47 ****
--- 41,47 ----
  #define unlink(path)          pgunlink(path)
  #endif
  
+ #ifdef WIN32
  extern int    copydir(char *fromdir, char *todir);
  
  /* Last parameter not used */
Index: src/port/dirmod.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/port/dirmod.c,v
retrieving revision 1.8
diff -c -c -r1.8 dirmod.c
*** src/port/dirmod.c   29 Nov 2003 19:52:13 -0000      1.8
--- src/port/dirmod.c   27 Jan 2004 03:46:08 -0000
***************
*** 27,35 ****
--- 27,45 ----
  {
        int                     loops = 0;
  
+ #ifdef WIN32
        while (!MoveFileEx(from, to, MOVEFILE_REPLACE_EXISTING))
+ #endif
+ #ifdef CYGWIN
+       while (rename(from, to) < 0)
+ #endif
        {
+ #ifdef WIN32
                if (GetLastError() != ERROR_ACCESS_DENIED)
+ #endif
+ #ifdef CYGWIN
+               if (errno != EACCES)
+ #endif
                        /* set errno? */
                        return -1;
                Sleep(100);                             /* ms */
---------------------------(end of broadcast)---------------------------
TIP 6: Have you searched our list archives?

               http://archives.postgresql.org

Reply via email to