Re: remove log_timestamp, log_pid and log_source_port GUC - Mailing list pgsql-patches

Patch applied.  Thanks.

---------------------------------------------------------------------------


Andrew Dunstan wrote:
> Bruce Momjian wrote:
>
> >Tom Lane wrote:
> >
> >
> >>Christopher Kings-Lynne <chriskl@familyhealth.com.au> writes:
> >>
> >>
> >>>>Please don't.  Declare them obsolete for 7.5 and remove them in a later
> >>>>release.
> >>>>
> >>>>
> >>>Nah, just remove them.  We've removed, added and changed so many config
> >>>options and no-one's ever complained...
> >>>
> >>>
> >>I agree with Chris; this would be taking compatibility concerns a bit
> >>far.  None of these variables are likely to be touched except through
> >>postgresql.conf (I don't think we even allow them to be SET interactively).
> >>And you can never simply take your old .conf file and plop it down into
> >>a new release.
> >>
> >>
> >
> >And the other reason to remove them is that if you don't, you will get
> >all sorts of confusion about people asking, "Which one should I use for
> >pid logging?  Looks like both work."
> >
> >
> >
> OK, Here's the patch. Also makes some minor improvements in the docco
> for log_line_prefix.
>
> cheers
>
> andrew

> Index: doc/src/sgml/runtime.sgml
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/doc/src/sgml/runtime.sgml,v
> retrieving revision 1.248
> diff -c -r1.248 runtime.sgml
> *** doc/src/sgml/runtime.sgml    9 Mar 2004 23:15:47 -0000    1.248
> --- doc/src/sgml/runtime.sgml    10 Mar 2004 16:07:46 -0000
> ***************
> *** 1993,1999 ****
>          <para>
>           Causes the duration of every completed statement to be logged.
>           To use this option, it is recommended that you also enable
> !         <varname>log_statement</> and <varname>log_pid</> so that you
>           can link the statement to the duration using the process
>           ID. The default is off.  Only superusers can turn off this
>           option if it is enabled by the administrator.
> --- 1993,2000 ----
>          <para>
>           Causes the duration of every completed statement to be logged.
>           To use this option, it is recommended that you also enable
> !         <varname>log_statement</> and if not using <application>syslog</>
> !      log the PID using <varname>log_line_prefix</> so that you
>           can link the statement to the duration using the process
>           ID. The default is off.  Only superusers can turn off this
>           option if it is enabled by the administrator.
> ***************
> *** 2014,2019 ****
> --- 2015,2022 ----
>        processes without controlling sessions. <application>Syslog</> produces its own
>        timestamp and process ID information, so you probably do not want to
>        use those escapes if you are using <application>syslog</>.
> +      This option can only be set at server start or in the
> +         <filename>postgresql.conf</filename> configuration file.
>        <informaltable>
>         <tgroup cols="3">
>          <thead>
> ***************
> *** 2058,2064 ****
>           <row>
>            <entry><literal>%c</literal></entry>
>            <entry>Session ID. A unique identifier for each session.
> !          It is 2 4-byte hexadecimal numbers separated by a dot. The numbers
>            are the Session Start Time and the Process ID, so this can also
>            be used as a space saving way of printing these items.</entry>
>            <entry>Yes</entry>
> --- 2061,2068 ----
>           <row>
>            <entry><literal>%c</literal></entry>
>            <entry>Session ID. A unique identifier for each session.
> !          It is 2 4-byte hexadecimal numbers (without leading zeros)
> !           separated by a dot. The numbers
>            are the Session Start Time and the Process ID, so this can also
>            be used as a space saving way of printing these items.</entry>
>            <entry>Yes</entry>
> ***************
> *** 2093,2111 ****
>         </listitem>
>        </varlistentry>
>
> -      <varlistentry>
> -       <term><varname>log_pid</varname> (<type>boolean</type>)</term>
> -       <listitem>
> -        <para>
> -         Prefixes each message in the server log file with the process ID of
> -         the server process. This is useful to sort out which messages
> -         pertain to which connection. The default is off.  This parameter
> -         does not affect messages logged via <application>syslog</>, which
> -         always contain the process ID.
> -        </para>
> -       </listitem>
> -      </varlistentry>
> -
>        <varlistentry id="guc-log-statement" xreflabel="log_statement">
>         <term><varname>log_statement</varname> (<type>boolean</type>)</term>
>         <listitem>
> --- 2097,2102 ----
> ***************
> *** 2120,2135 ****
>         </listitem>
>        </varlistentry>
>
> -      <varlistentry id="guc-log-timestamp" xreflabel="log_timestamp">
> -       <term><varname>log_timestamp</varname> (<type>boolean</type>)</term>
> -       <listitem>
> -        <para>
> -         Prefixes each server log message with a time stamp. The default
> -         is off.
> -        </para>
> -       </listitem>
> -      </varlistentry>
> -
>        <varlistentry id="guc-log-hostname" xreflabel="log_hostname">
>         <term><varname>log_hostname</varname> (<type>boolean</type>)</term>
>         <listitem>
> --- 2111,2116 ----
> ***************
> *** 2143,2161 ****
>         </listitem>
>        </varlistentry>
>
> -      <varlistentry id="guc-log-source-port" xreflabel="log_source_port">
> -       <term><varname>log_source_port</varname> (<type>boolean</type>)</term>
> -       <listitem>
> -        <para>
> -         Shows the outgoing port number of the connecting host in the
> -         connection log messages. You could trace back the port number
> -         to find out what user initiated the connection. Other than
> -         that, it's pretty useless and therefore off by default. This
> -         option can only be set at server start.
> -        </para>
> -       </listitem>
> -      </varlistentry>
> -
>        </variablelist>
>       </sect3>
>      </sect2>
> --- 2124,2129 ----
> Index: src/backend/postmaster/postmaster.c
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/backend/postmaster/postmaster.c,v
> retrieving revision 1.372
> diff -c -r1.372 postmaster.c
> *** src/backend/postmaster/postmaster.c    9 Mar 2004 05:11:52 -0000    1.372
> --- src/backend/postmaster/postmaster.c    10 Mar 2004 16:07:46 -0000
> ***************
> *** 212,219 ****
>   int            CheckPointWarning = 30;
>   time_t        LastSignalledCheckpoint = 0;
>
> ! bool        log_hostname;        /* for ps display */
> ! bool        LogSourcePort;
>   bool        Log_connections = false;
>   bool        Db_user_namespace = false;
>
> --- 212,218 ----
>   int            CheckPointWarning = 30;
>   time_t        LastSignalledCheckpoint = 0;
>
> ! bool        log_hostname;        /* for ps display and logging */
>   bool        Log_connections = false;
>   bool        Db_user_namespace = false;
>
> ***************
> *** 2419,2424 ****
> --- 2418,2424 ----
>       struct timezone tz;
>       char        remote_host[NI_MAXHOST];
>       char        remote_port[NI_MAXSERV];
> +     char        remote_ps_data[NI_MAXHOST];
>
>       IsUnderPostmaster = true;    /* we are a postmaster subprocess now */
>
> ***************
> *** 2479,2499 ****
>                           remote_port, sizeof(remote_port),
>                           NI_NUMERICHOST | NI_NUMERICSERV);
>       }
>
>       if (Log_connections)
>           ereport(LOG,
>                   (errmsg("connection received: host=%s port=%s",
>                           remote_host, remote_port)));
>
> -     if (LogSourcePort)
> -     {
> -         /* modify remote_host for use in ps status */
> -         char        tmphost[NI_MAXHOST];
> -
> -         snprintf(tmphost, sizeof(tmphost), "%s(%s)", remote_host, remote_port);
> -         StrNCpy(remote_host, tmphost, sizeof(remote_host));
> -     }
> -
>       /*
>        * save remote_host and remote_port in port stucture
>        */
> --- 2479,2493 ----
>                           remote_port, sizeof(remote_port),
>                           NI_NUMERICHOST | NI_NUMERICSERV);
>       }
> +     snprintf(remote_ps_data, sizeof(remote_ps_data),
> +              remote_port[0] == '\0' ? "%s" : "%s(%s)",
> +              remote_host, remote_port);
>
>       if (Log_connections)
>           ereport(LOG,
>                   (errmsg("connection received: host=%s port=%s",
>                           remote_host, remote_port)));
>
>       /*
>        * save remote_host and remote_port in port stucture
>        */
> ***************
> *** 2522,2528 ****
>        * title for ps.  It's good to do this as early as possible in
>        * startup.
>        */
> !     init_ps_display(port->user_name, port->database_name, remote_host);
>       set_ps_display("authentication");
>
>       /*
> --- 2516,2522 ----
>        * title for ps.  It's good to do this as early as possible in
>        * startup.
>        */
> !     init_ps_display(port->user_name, port->database_name, remote_ps_data);
>       set_ps_display("authentication");
>
>       /*
> Index: src/backend/tcop/postgres.c
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/backend/tcop/postgres.c,v
> retrieving revision 1.394
> diff -c -r1.394 postgres.c
> *** src/backend/tcop/postgres.c    9 Mar 2004 04:43:07 -0000    1.394
> --- src/backend/tcop/postgres.c    10 Mar 2004 16:07:47 -0000
> ***************
> *** 3212,3222 ****
>       snprintf(dbname, sizeof(dbname)," database=%s",port->database_name);
>       snprintf(remote_host,sizeof(remote_host)," host=%s",
>                port->remote_host);
> !     /* prevent redundant or empty reporting of port */
> !     if (!LogSourcePort && strlen(port->remote_port))
> !         snprintf(remote_port,sizeof(remote_port)," port=%s",port->remote_port);
> !     else
> !         remote_port[0] = '\0';
>
>
>       gettimeofday(&end,NULL);
> --- 3212,3218 ----
>       snprintf(dbname, sizeof(dbname)," database=%s",port->database_name);
>       snprintf(remote_host,sizeof(remote_host)," host=%s",
>                port->remote_host);
> !     snprintf(remote_port,sizeof(remote_port)," port=%s",port->remote_port);
>
>
>       gettimeofday(&end,NULL);
> Index: src/backend/utils/error/elog.c
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/backend/utils/error/elog.c,v
> retrieving revision 1.127
> diff -c -r1.127 elog.c
> *** src/backend/utils/error/elog.c    9 Mar 2004 04:43:07 -0000    1.127
> --- src/backend/utils/error/elog.c    10 Mar 2004 16:07:47 -0000
> ***************
> *** 69,77 ****
>
>   /* GUC parameters */
>   PGErrorVerbosity Log_error_verbosity = PGERROR_VERBOSE;
> - bool        Log_timestamp = false;        /* show timestamps in stderr
> -                                          * output */
> - bool        Log_pid = false;    /* show PIDs in stderr output */
>   char       *Log_line_prefix = ""; /* format for extra log line info */
>
>   #ifdef HAVE_SYSLOG
> --- 69,74 ----
> ***************
> *** 144,151 ****
>   static char *expand_fmt_string(const char *fmt, ErrorData *edata);
>   static const char *useful_strerror(int errnum);
>   static const char *error_severity(int elevel);
> - static const char *print_timestamp(void);
> - static const char *print_pid(void);
>   static void append_with_tabs(StringInfo buf, const char *str);
>   static const char *log_line_prefix(void);
>
> --- 141,146 ----
> ***************
> *** 1134,1140 ****
>                   case 'r':
>                       j += snprintf(result+j,result_len-j,"%s",
>                                     MyProcPort->remote_host);
> !                     if (!LogSourcePort && strlen(MyProcPort->remote_port))
>                           j += snprintf(result+j,result_len-j,"(%s)",
>                                         MyProcPort->remote_port);
>                       break;
> --- 1129,1135 ----
>                   case 'r':
>                       j += snprintf(result+j,result_len-j,"%s",
>                                     MyProcPort->remote_host);
> !                     if (strlen(MyProcPort->remote_port) > 0)
>                           j += snprintf(result+j,result_len-j,"(%s)",
>                                         MyProcPort->remote_port);
>                       break;
> ***************
> *** 1293,1302 ****
>            * Timestamp and PID are only used for stderr output --- we assume
>            * the syslog daemon will supply them for us in the other case.
>            */
> !         fprintf(stderr, "%s%s%s",
> !                 Log_timestamp ? print_timestamp() : "",
> !                 Log_pid ? print_pid() : "",
> !                 buf.data);
>       }
>
>       pfree(buf.data);
> --- 1288,1294 ----
>            * Timestamp and PID are only used for stderr output --- we assume
>            * the syslog daemon will supply them for us in the other case.
>            */
> !         fprintf(stderr, "%s",buf.data);
>       }
>
>       pfree(buf.data);
> ***************
> *** 1567,1609 ****
>       return prefix;
>   }
>
> -
> - /*
> -  * Return a timestamp string like
> -  *
> -  *     "2000-06-04 13:12:03 "
> -  */
> - static const char *
> - print_timestamp(void)
> - {
> -     time_t        curtime;
> -     static char buf[21];        /* format `YYYY-MM-DD HH:MM:SS ' */
> -
> -     curtime = time(NULL);
> -
> -     strftime(buf, sizeof(buf),
> -              "%Y-%m-%d %H:%M:%S ",
> -              localtime(&curtime));
> -
> -     return buf;
> - }
> -
> -
> - /*
> -  * Return a string like
> -  *
> -  *       "[123456] "
> -  *
> -  * with the current pid.
> -  */
> - static const char *
> - print_pid(void)
> - {
> -     static char buf[10];        /* allow `[123456] ' */
> -
> -     snprintf(buf, sizeof(buf), "[%d] ", (int) MyProcPid);
> -     return buf;
> - }
>
>   /*
>    *    append_with_tabs
> --- 1559,1564 ----
> Index: src/backend/utils/misc/guc.c
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/backend/utils/misc/guc.c,v
> retrieving revision 1.189
> diff -c -r1.189 guc.c
> *** src/backend/utils/misc/guc.c    9 Mar 2004 04:43:07 -0000    1.189
> --- src/backend/utils/misc/guc.c    10 Mar 2004 16:07:49 -0000
> ***************
> *** 508,529 ****
>           &Log_disconnections,
>           false, NULL, NULL
>       },
> -     {
> -         {"log_timestamp", PGC_SIGHUP, LOGGING_WHAT,
> -             gettext_noop("Prefixes server log messages with a time stamp."),
> -             NULL
> -         },
> -         &Log_timestamp,
> -         false, NULL, NULL
> -     },
> -     {
> -         {"log_pid", PGC_SIGHUP, LOGGING_WHAT,
> -             gettext_noop("Prefixes server log messages with the server PID."),
> -             NULL
> -         },
> -         &Log_pid,
> -         false, NULL, NULL
> -     },
>
>   #ifdef USE_ASSERT_CHECKING
>       {
> --- 508,513 ----
> ***************
> *** 750,764 ****
>           &log_hostname,
>           false, NULL, NULL
>       },
> -     {
> -         {"log_source_port", PGC_SIGHUP, LOGGING_WHAT,
> -             gettext_noop("Logs the outgoing port number of the connecting host."),
> -             NULL
> -         },
> -         &LogSourcePort,
> -         false, NULL, NULL
> -     },
> -
>       {
>           {"sql_inheritance", PGC_USERSET, COMPAT_OPTIONS_PREVIOUS,
>               gettext_noop("Causes subtables to be included by default in various commands."),
> --- 734,739 ----
> Index: src/backend/utils/misc/postgresql.conf.sample
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/backend/utils/misc/postgresql.conf.sample,v
> retrieving revision 1.107
> diff -c -r1.107 postgresql.conf.sample
> *** src/backend/utils/misc/postgresql.conf.sample    9 Mar 2004 04:43:07 -0000    1.107
> --- src/backend/utils/misc/postgresql.conf.sample    10 Mar 2004 16:07:49 -0000
> ***************
> *** 184,190 ****
>   #log_connections = false
>   #log_disconnections = false
>   #log_duration = false
> - #log_pid = false
>   #log_line_prefix = ''        # e.g. '<%u%%%d> '
>                   # %u=user name %d=database name
>                   # %r=remote host and port
> --- 184,189 ----
> ***************
> *** 194,202 ****
>                   # %x=stop here in non-session processes
>                   # %%='%'
>   #log_statement = false
> - #log_timestamp = false
>   #log_hostname = false
> - #log_source_port = false
>
>
>   #---------------------------------------------------------------------------
> --- 193,199 ----
> Index: src/include/tcop/tcopprot.h
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/include/tcop/tcopprot.h,v
> retrieving revision 1.61
> diff -c -r1.61 tcopprot.h
> *** src/include/tcop/tcopprot.h    29 Nov 2003 22:41:14 -0000    1.61
> --- src/include/tcop/tcopprot.h    10 Mar 2004 16:07:50 -0000
> ***************
> *** 30,36 ****
>   extern bool InError;
>   extern CommandDest whereToSendOutput;
>   extern bool log_hostname;
> - extern bool LogSourcePort;
>   extern DLLIMPORT const char *debug_query_string;
>   extern char *rendezvous_name;
>
> --- 30,35 ----
> Index: src/include/utils/elog.h
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/include/utils/elog.h,v
> retrieving revision 1.65
> diff -c -r1.65 elog.h
> *** src/include/utils/elog.h    9 Mar 2004 04:43:07 -0000    1.65
> --- src/include/utils/elog.h    10 Mar 2004 16:07:50 -0000
> ***************
> *** 169,176 ****
>   } PGErrorVerbosity;
>
>   extern PGErrorVerbosity Log_error_verbosity;
> - extern bool Log_timestamp;
> - extern bool Log_pid;
>   extern char *Log_line_prefix;
>
>   #ifdef HAVE_SYSLOG
> --- 169,174 ----

>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: Have you searched our list archives?
>
>                http://archives.postgresql.org

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073