Re: [PATCHES] COPYable logs - Mailing list pgsql-hackers

Tom Lane wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
>
>> If these prove difficult, I'd say 24K would put us in an equivalent
>> position (two extra copies of the error message plus change). Even so,
>> I'm inclined to say that 8K is very tight.
>>
>
> We really only care about being able to deliver an "out of memory during
> error recovery" message within that space.  So yes, you can assume the
> message text isn't huge and there isn't any add-on context info to worry
> about.  But there could be localization and/or encoding translation costs.
>
>
>

Well, the attached small patch should reduce the memory impact
substantially. Not sure whether you think that's sufficient, or you want
to take it further.

cheers

andrew
Index: src/backend/utils/error/elog.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/error/elog.c,v
retrieving revision 1.194
diff -c -r1.194 elog.c
*** src/backend/utils/error/elog.c    19 Aug 2007 01:41:25 -0000    1.194
--- src/backend/utils/error/elog.c    20 Aug 2007 01:34:38 -0000
***************
*** 133,139 ****
  static void append_with_tabs(StringInfo buf, const char *str);
  static bool is_log_level_output(int elevel, int log_min_level);
  static void write_pipe_chunks(char *data, int len, int dest);
! static void get_error_message(StringInfo buf, ErrorData *edata);

  /*
   * errstart --- begin an error-reporting cycle
--- 133,140 ----
  static void append_with_tabs(StringInfo buf, const char *str);
  static bool is_log_level_output(int elevel, int log_min_level);
  static void write_pipe_chunks(char *data, int len, int dest);
! static void get_csv_error_message(StringInfo buf, ErrorData *edata);
! static void write_csvlog(ErrorData *edata);

  /*
   * errstart --- begin an error-reporting cycle
***************
*** 1809,1817 ****
      appendStringInfoChar(&buf, ',');

      /* Error message and cursor position if any */
!     get_error_message(&msgbuf, edata);
!
!     appendCSVLiteral(&buf, msgbuf.data);

      appendStringInfoChar(&buf, '\n');

--- 1810,1816 ----
      appendStringInfoChar(&buf, ',');

      /* Error message and cursor position if any */
!     get_csv_error_message(&buf, edata);

      appendStringInfoChar(&buf, '\n');

***************
*** 1826,1840 ****
  }

  /*
!  * Appends the buffer with the error message and the cursor position.
   */
  static void
! get_error_message(StringInfo buf, ErrorData *edata)
  {
!     if (edata->message)
!         appendStringInfo(buf, "%s", edata->message);
!     else
!         appendStringInfo(buf, "%s", _("missing error text"));

      if (edata->cursorpos > 0)
          appendStringInfo(buf, _(" at character %d"),
--- 1825,1847 ----
  }

  /*
!  * Appends the buffer with the error message and the cursor position, all
!  * CSV escaped.
   */
  static void
! get_csv_error_message(StringInfo buf, ErrorData *edata)
  {
!     char *msg = edata->message ? edata-> message : _("missing error text");
!     char c;
!
!     appendStringInfoCharMacro(buf, '"');
!
!     while ( (c = *msg++) != '\0' )
!     {
!       if (c == '"')
!           appendStringInfoCharMacro(buf, '"');
!       appendStringInfoCharMacro(buf, c);
!     }

      if (edata->cursorpos > 0)
          appendStringInfo(buf, _(" at character %d"),
***************
*** 1842,1847 ****
--- 1849,1856 ----
      else if (edata->internalpos > 0)
          appendStringInfo(buf, _(" at character %d"),
                           edata->internalpos);
+
+     appendStringInfoCharMacro(buf, '"');
  }

  /*
***************
*** 2032,2044 ****
              write(fileno(stderr), buf.data, buf.len);
      }

      if (Log_destination & LOG_DESTINATION_CSVLOG)
      {
          if (redirection_done || am_syslogger)
          {
              /* send CSV data if it's safe to do so (syslogger doesn't need
!              * the pipe)
               */
              write_csvlog(edata);
          }
          else
--- 2041,2059 ----
              write(fileno(stderr), buf.data, buf.len);
      }

+     /* If in the syslogger process, try to write messages direct to file */
+     if (am_syslogger)
+         write_syslogger_file(buf.data, buf.len, LOG_DESTINATION_STDERR);
+
+     /* Write to CSV log if enabled */
      if (Log_destination & LOG_DESTINATION_CSVLOG)
      {
          if (redirection_done || am_syslogger)
          {
              /* send CSV data if it's safe to do so (syslogger doesn't need
!              * the pipe). First get back the space in the message buffer.
               */
+             pfree(buf.data);
              write_csvlog(edata);
          }
          else
***************
*** 2051,2064 ****
                  /* write message to stderr unless we just sent it above */
                  write(fileno(stderr), buf.data, buf.len);
              }
          }
      }
!
!     /* If in the syslogger process, try to write messages direct to file */
!     if (am_syslogger)
!         write_syslogger_file(buf.data, buf.len, LOG_DESTINATION_STDERR);
!
!     pfree(buf.data);
  }

  /*
--- 2066,2078 ----
                  /* write message to stderr unless we just sent it above */
                  write(fileno(stderr), buf.data, buf.len);
              }
+             pfree(buf.data);
          }
      }
!     else
!     {
!         pfree(buf.data);
!     }
  }

  /*