Re: Win32 testing needed

Tom Lane wrote:
> I have just committed a somewhat trimmed-down version of Andreas Pflug's
> syslogger patch.  It seems to work okay on Unix (with or without
> EXEC_BACKEND) but I'm not in a position to test it on Windows.  Would
> someone give it a try and report back?  Please check in particular that
> the logger shuts down cleanly after the postmaster is gone.

Attached a patch with several issues resolved; only win32 checked.

After your changes, the error from ReadFile is not ERROR_HANDLE_EOF any 
more, but ERROR_PIPE_BROKEN (which should be expected either), check is 
done for both now.

The logger should *not* use proc_exit but exit(0), because proc_exit 
might try to elog something, after we just closed the log file. IMHO 
there's nothing left to cleanup anyway.

Changing setvbuf to use line buffered mode broke win32; apparently a 
line is not a line there.... changed back to _IONBUF which should be 
identical in result because we're always writing a complete line.

An observation I didn't track down so far:
Some LOG messages (e.g. the final logger shutdown, or "received fast 
shutdown request") don't have proper CRLF line ending in win32, but only 
LF.

If the logger subprocess is killed, it will come up again ok, but 
redirecting to NULL_DEV doesn't work (open returns -1; that's what I had 
realStdErr for). Opening c:\temp\dummy instead does help, but that's not 
a solution. So what now? I'd propose inheriting the old stderr handle 
instead of redirection_done so it can be reused in this case, as in my 
original posting.

Finally, you don't seem to be a friend of a logfile rotation user 
trigger... Please consider including it anyway.



Regards,
Andreas
Index: backend/postmaster/postmaster.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/backend/postmaster/postmaster.c,v
retrieving revision 1.420
diff -u -r1.420 postmaster.c
--- backend/postmaster/postmaster.c    5 Aug 2004 23:32:10 -0000    1.420
+++ backend/postmaster/postmaster.c    6 Aug 2004 10:01:57 -0000
@@ -3082,7 +3082,11 @@
              */
             kill(PgArchPID, SIGUSR1);
         }
-    }
+        }
+        if (CheckPostmasterSignal(PMSIGNAL_ROTATE_LOGFILE) && SysLoggerPID != 0)
+        {
+                kill(SysLoggerPID, SIGUSR1);
+        }
 
     PG_SETMASK(&UnBlockSig);
 
Index: backend/postmaster/syslogger.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/backend/postmaster/syslogger.c,v
retrieving revision 1.1
diff -u -r1.1 syslogger.c
--- backend/postmaster/syslogger.c    5 Aug 2004 23:32:10 -0000    1.1
+++ backend/postmaster/syslogger.c    6 Aug 2004 10:01:57 -0000
@@ -37,6 +37,7 @@
 #include "pgtime.h"
 #include "storage/ipc.h"
 #include "storage/pg_shmem.h"
+#include "storage/pmsignal.h"
 #include "utils/guc.h"
 #include "utils/ps_status.h"
 
@@ -83,6 +84,7 @@
  * Flags set by interrupt handlers for later service in the main loop.
  */
 static volatile sig_atomic_t got_SIGHUP = false;
+static volatile sig_atomic_t rotation_requested = false;
 
 
 /* Local subroutines */
@@ -96,6 +98,7 @@
 static void logfile_rotate(void);
 static char* logfile_getname(pg_time_t timestamp);
 static void sigHupHandler(SIGNAL_ARGS);
+static void rotationHandler(SIGNAL_ARGS);
 
 
 /*
@@ -168,7 +171,7 @@
     pqsignal(SIGQUIT, SIG_IGN);
     pqsignal(SIGALRM, SIG_IGN);
     pqsignal(SIGPIPE, SIG_IGN);
-    pqsignal(SIGUSR1, SIG_IGN);
+    pqsignal(SIGUSR1, rotationHandler);
     pqsignal(SIGUSR2, SIG_IGN);
 
     /*
@@ -201,7 +204,6 @@
     /* main worker loop */
     for (;;)
     {
-        bool rotation_requested = false;
 #ifndef WIN32
         char        logbuffer[1024];
         int          bytesRead;
@@ -255,7 +257,10 @@
         }
 
         if (rotation_requested)
-            logfile_rotate();
+                {
+            logfile_rotate();            
+                        rotation_requested = false;
+                }
 
 #ifndef WIN32
         /*
@@ -320,7 +325,7 @@
             if (syslogFile)
                 fclose(syslogFile);
             /* normal exit from the syslogger is here */
-            proc_exit(0);
+            exit(0);
         }
     }
 }
@@ -401,7 +406,7 @@
                  (errmsg("could not create logfile \"%s\": %m",
                          filename))));
 
-    setvbuf(syslogFile, NULL, _IOLBF, 0);
+    setvbuf(syslogFile, NULL, _IONBF, 0);
 
     pfree(filename);
 
@@ -557,7 +562,7 @@
     if (fd != -1)
     {
         syslogFile = fdopen(fd, "a");
-        setvbuf(syslogFile, NULL, _IOLBF, 0);
+        setvbuf(syslogFile, NULL, _IONBF, 0);
     }
     redirection_done = (bool) atoi(*argv++);
 #else  /* WIN32 */
@@ -568,7 +573,7 @@
         if (fd != 0)
         {
             syslogFile = fdopen(fd, "a");
-            setvbuf(syslogFile, NULL, _IOLBF, 0);
+            setvbuf(syslogFile, NULL, _IONBF, 0);
         }
     }
     redirection_done = (bool) atoi(*argv++);
@@ -631,11 +636,11 @@
         {
             DWORD error = GetLastError();
 
-            if (error == ERROR_HANDLE_EOF)
+            if (error == ERROR_HANDLE_EOF || error == ERROR_BROKEN_PIPE)
                 break;
             ereport(LOG,
                     (errcode_for_file_access(),
-                     errmsg("could not read from logger pipe: %m")));
+                     errmsg("could not read from logger pipe: %lu", error)));
         }
         else if (bytesRead > 0)
             write_syslogger_file(logbuffer, bytesRead);
@@ -689,7 +694,7 @@
         return;
     }
 
-    setvbuf(fh, NULL, _IOLBF, 0);
+    setvbuf(fh, NULL, _IONBF, 0);
 
     /* On Windows, need to interlock against data-transfer thread */
 #ifdef WIN32
@@ -735,6 +740,26 @@
     return filename;
 }
 
+
+/*
+ * Rotate log file
+ */
+bool
+LogFileRotate(void)
+{
+        if (!(Redirect_stderr))
+        {
+                ereport(NOTICE,
+                (errcode(ERRCODE_WARNING),
+                 errmsg("no logfile configured; rotation not supported")));
+                return false;
+        }
+
+        SendPostmasterSignal(PMSIGNAL_ROTATE_LOGFILE);
+
+        return true;
+}
+
 /* --------------------------------
  *        signal handler routines
  * --------------------------------
@@ -746,3 +771,10 @@
 {
     got_SIGHUP = true;
 }
+
+/* SIGUSR1: set flag to rotate logfile */
+static void
+rotationHandler(SIGNAL_ARGS)
+{
+    rotation_requested = true;
+}
Index: include/postmaster/syslogger.h
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/include/postmaster/syslogger.h,v
retrieving revision 1.1
diff -u -r1.1 syslogger.h
--- include/postmaster/syslogger.h    5 Aug 2004 23:32:12 -0000    1.1
+++ include/postmaster/syslogger.h    6 Aug 2004 10:01:59 -0000
@@ -32,6 +32,8 @@
 
 extern void write_syslogger_file(const char *buffer, int count);
 
+extern bool LogFileRotate(void);
+
 #ifdef EXEC_BACKEND
 extern void SysLoggerMain(int argc, char *argv[]);
 #endif
Index: include/storage/pmsignal.h
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/include/storage/pmsignal.h,v
retrieving revision 1.9
diff -u -r1.9 pmsignal.h
--- include/storage/pmsignal.h    19 Jul 2004 02:47:15 -0000    1.9
+++ include/storage/pmsignal.h    6 Aug 2004 10:01:59 -0000
@@ -25,7 +25,7 @@
     PMSIGNAL_PASSWORD_CHANGE,    /* pg_pwd file has changed */
     PMSIGNAL_WAKEN_CHILDREN,    /* send a SIGUSR1 signal to all backends */
     PMSIGNAL_WAKEN_ARCHIVER,    /* send a NOTIFY signal to xlog archiver */
-
+    PMSIGNAL_ROTATE_LOGFILE,    /* send SIGUSR1 to syslogger to rotate logfile */
     NUM_PMSIGNALS                /* Must be last value of enum! */
 } PMSignalReason;