Re: write_pipe_chunks patch messes up early error message output - Mailing list pgsql-hackers

Tom Lane wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
>
>> Or, looking at it another way, why would we ever want the syslogger to
>> use the chunking protocol at all?
>>
>
> Ah, I misunderstood you.  Yeah, I think you are right: if we are
> special-casing the syslogger process anyway, then it need only have
> these two behaviors:
>
> not redirection_done: write to own stderr (not chunked) and directly to
> file
>
> redirection_done: write directly to file
>
> One thing to watch out for is infinite recursion if the write-to-file
> gets an error.  I don't remember if we have a defense against that
> in there now, but we probably should.
>
>
>

I think we do. write_syslogger_file says:

    /* can't use ereport here because of possible recursion */

Anyway, I think the attached patch will do what we need.

cheers

andrew
Index: src/backend/postmaster/postmaster.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/postmaster/postmaster.c,v
retrieving revision 1.532
diff -c -r1.532 postmaster.c
*** src/backend/postmaster/postmaster.c    11 Jul 2007 08:27:33 -0000    1.532
--- src/backend/postmaster/postmaster.c    19 Jul 2007 15:53:39 -0000
***************
*** 203,210 ****
              BgWriterPID = 0,
              AutoVacPID = 0,
              PgArchPID = 0,
!             PgStatPID = 0;
! pid_t            SysLoggerPID = 0; /* Needs to be accessed from elog.c */

  /* Startup/shutdown state */
  #define            NoShutdown        0
--- 203,210 ----
              BgWriterPID = 0,
              AutoVacPID = 0,
              PgArchPID = 0,
!             PgStatPID = 0,
!             SysLoggerPID = 0;

  /* Startup/shutdown state */
  #define            NoShutdown        0
***************
*** 218,223 ****
--- 218,225 ----
  bool        ClientAuthInProgress = false;        /* T during new-client
                                                   * authentication */

+ bool redirection_done = false;
+
  /* received START_AUTOVAC_LAUNCHER signal */
  static bool start_autovac_launcher = false;

***************
*** 332,337 ****
--- 334,340 ----
      InheritableSocket pgStatSock;
      pid_t        PostmasterPid;
      TimestampTz PgStartTime;
+     bool        redirection_done;
  #ifdef WIN32
      HANDLE        PostmasterHandle;
      HANDLE        initial_signal_pipe;
***************
*** 3953,3958 ****
--- 3956,3963 ----
      param->PostmasterPid = PostmasterPid;
      param->PgStartTime = PgStartTime;

+     param->redirection_done = redirection_done;
+
  #ifdef WIN32
      param->PostmasterHandle = PostmasterHandle;
      write_duplicated_handle(¶m->initial_signal_pipe,
***************
*** 4156,4161 ****
--- 4161,4168 ----
      PostmasterPid = param->PostmasterPid;
      PgStartTime = param->PgStartTime;

+     redirection_done = param->redirection_done;
+
  #ifdef WIN32
      PostmasterHandle = param->PostmasterHandle;
      pgwin32_initial_signal_pipe = param->initial_signal_pipe;
Index: src/backend/postmaster/syslogger.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/postmaster/syslogger.c,v
retrieving revision 1.32
diff -c -r1.32 syslogger.c
*** src/backend/postmaster/syslogger.c    14 Jun 2007 01:48:51 -0000    1.32
--- src/backend/postmaster/syslogger.c    19 Jul 2007 15:53:39 -0000
***************
*** 79,89 ****
   */
  bool        am_syslogger = false;

  /*
   * Private state
   */
  static pg_time_t next_rotation_time;
- static bool redirection_done = false;
  static bool pipe_eof_seen = false;
  static FILE *syslogFile = NULL;
  static char *last_file_name = NULL;
--- 79,90 ----
   */
  bool        am_syslogger = false;

+ extern bool redirection_done;
+
  /*
   * Private state
   */
  static pg_time_t next_rotation_time;
  static bool pipe_eof_seen = false;
  static FILE *syslogFile = NULL;
  static char *last_file_name = NULL;
***************
*** 582,595 ****
          snprintf(numbuf[bufc++], 32, "%d", fileno(syslogFile));
      else
          strcpy(numbuf[bufc++], "-1");
-     snprintf(numbuf[bufc++], 32, "%d", (int) redirection_done);
  #else                            /* WIN32 */
      if (syslogFile != NULL)
          snprintf(numbuf[bufc++], 32, "%ld",
                   _get_osfhandle(_fileno(syslogFile)));
      else
          strcpy(numbuf[bufc++], "0");
-     snprintf(numbuf[bufc++], 32, "%d", (int) redirection_done);
  #endif   /* WIN32 */

      /* Add to the arg list */
--- 583,594 ----
***************
*** 623,629 ****
          syslogFile = fdopen(fd, "a");
          setvbuf(syslogFile, NULL, LBF_MODE, 0);
      }
-     redirection_done = (bool) atoi(*argv++);
  #else                            /* WIN32 */
      fd = atoi(*argv++);
      if (fd != 0)
--- 622,627 ----
***************
*** 635,641 ****
              setvbuf(syslogFile, NULL, LBF_MODE, 0);
          }
      }
-     redirection_done = (bool) atoi(*argv++);
  #endif   /* WIN32 */
  }
  #endif   /* EXEC_BACKEND */
--- 633,638 ----
Index: src/backend/utils/error/elog.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/error/elog.c,v
retrieving revision 1.187
diff -c -r1.187 elog.c
*** src/backend/utils/error/elog.c    14 Jun 2007 01:48:51 -0000    1.187
--- src/backend/utils/error/elog.c    19 Jul 2007 15:53:41 -0000
***************
*** 76,82 ****

  sigjmp_buf *PG_exception_stack = NULL;

! extern pid_t SysLoggerPID;

  /* GUC parameters */
  PGErrorVerbosity Log_error_verbosity = PGERROR_VERBOSE;
--- 76,82 ----

  sigjmp_buf *PG_exception_stack = NULL;

! extern bool redirection_done;

  /* GUC parameters */
  PGErrorVerbosity Log_error_verbosity = PGERROR_VERBOSE;
***************
*** 1780,1790 ****
           * that's really a pipe to the syslogger process. Unless we're in the
           * postmaster, and the syslogger process isn't started yet.
           */
!         if ((!Redirect_stderr || am_syslogger || (!IsUnderPostmaster && SysLoggerPID==0)) && pgwin32_is_service())
              write_eventlog(edata->elevel, buf.data);
          else
  #endif
!             if (Redirect_stderr)
                  write_pipe_chunks(fileno(stderr), buf.data, buf.len);
              else
                  write(fileno(stderr), buf.data, buf.len);
--- 1780,1795 ----
           * that's really a pipe to the syslogger process. Unless we're in the
           * postmaster, and the syslogger process isn't started yet.
           */
!         if (pgwin32_is_service &&
!             (!Redirect_stderr || !redirection_done || am_syslogger))
              write_eventlog(edata->elevel, buf.data);
          else
  #endif
!             /* only use the chunking protocol if we know the syslogger should
!              * be catching stderr output, and we are not ourselves the
!              * syslogger. Otherwise, go directly to stderr.
!              */
!             if (Redirect_stderr && redirection_done && !am_syslogger)
                  write_pipe_chunks(fileno(stderr), buf.data, buf.len);
              else
                  write(fileno(stderr), buf.data, buf.len);