Thread: cleanup log_disconnections()
Trivial patch to cleanup log_disconnections() function.
Regards,
Qingqing
---
Index: postgres.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/tcop/postgres.c,v
retrieving revision 1.463
diff -u -r1.463 postgres.c
--- postgres.c 26 Sep 2005 15:51:12 -0000 1.463
+++ postgres.c 3 Oct 2005 04:34:17 -0000
@@ -3526,29 +3526,13 @@
static void
log_disconnections(int code, Datum arg)
{
- Port *port = MyProcPort;
- struct timeval end;
- int hours,
- minutes,
- seconds;
+ Port *port = MyProcPort;
+ struct timeval end;
+ int hours,
+ minutes,
+ seconds;
- char session_time[20];
- char uname[6 + NAMEDATALEN];
- char dbname[10 + NAMEDATALEN];
- char remote_host[7 + NI_MAXHOST];
- char remote_port[7 + NI_MAXSERV];
-
- snprintf(uname, sizeof(uname), " user=%s", port->user_name);
- snprintf(dbname, sizeof(dbname), " database=%s", port->database_name);
- snprintf(remote_host, sizeof(remote_host), " host=%s",
- port->remote_host);
- if (port->remote_port[0])
- snprintf(remote_port, sizeof(remote_port), " port=%s", port->remote_port);
- else
- remote_port[0] = '\0';
-
gettimeofday(&end, NULL);
-
if (end.tv_usec < port->session_start.tv_usec)
{
end.tv_sec--;
@@ -3557,28 +3541,16 @@
end.tv_sec -= port->session_start.tv_sec;
end.tv_usec -= port->session_start.tv_usec;
+ /* for stricter accuracy here we could round - this is close enough */
hours = end.tv_sec / SECS_PER_HOUR;
end.tv_sec %= SECS_PER_HOUR;
minutes = end.tv_sec / SECS_PER_MINUTE;
seconds = end.tv_sec % SECS_PER_MINUTE;
- /* if time has gone backwards for some reason say so, or print time */
-
- if (end.tv_sec < 0)
- snprintf(session_time, sizeof(session_time), "negative!");
- else
-
- /*
- * for stricter accuracy here we could round - this is close
- * enough
- */
- snprintf(session_time, sizeof(session_time),
- "%d:%02d:%02d.%02d",
- hours, minutes, seconds, (int) (end.tv_usec / 10000));
-
- ereport(
- LOG,
- (errmsg("disconnection: session time: %s%s%s%s%s",
- session_time, uname, dbname, remote_host, remote_port)));
-
+ ereport(LOG,
+ (errmsg("disconnection: session time=%d:%02d:%02d.%02d,"
+ " user=%s, database=%s, host=%s, port=%s",
+ hours, minutes, seconds, (int) (end.tv_usec / 10000),
+ port->user_name, port->database_name, port->remote_host,
+ port->remote_port[0]?port->remote_port : "<unknown>")));
}
On Mon, 2005-03-10 at 00:37 -0400, Qingqing Zhou wrote: > Trivial patch to cleanup log_disconnections() function. Why does the patch change behavior when - port->remote_port is the empty string - or, when end.tv_sec is negative I'm not adamant that the old behavior is better, but I'd like to see some justification for why the changes are improvements. -Neil
On Mon, 3 Oct 2005, Neil Conway wrote:
> Why does the patch change behavior when
>
> - port->remote_port is the empty string
Ooops, this wording change is inconsistent with Log_connections, so I
reverted it. Updated patch is attached.
> - or, when end.tv_sec is negative
>
I can only imagine end.tv_sec becomes negative if the root changes system
time during the connection. We can't prevent this, and it is not our
problem to detect this (in fact, we can't), so just tell the truth
directly. If we detect this here, other places in the code should detect
this as well.
Thanks for pointing out, Qingqing
---
Index: postgres.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/tcop/postgres.c,v
retrieving revision 1.463
diff -u -r1.463 postgres.c
--- postgres.c 26 Sep 2005 15:51:12 -0000 1.463
+++ postgres.c 3 Oct 2005 16:20:01 -0000
@@ -3526,29 +3526,13 @@
static void
log_disconnections(int code, Datum arg)
{
- Port *port = MyProcPort;
- struct timeval end;
- int hours,
- minutes,
- seconds;
+ Port *port = MyProcPort;
+ struct timeval end;
+ int hours,
+ minutes,
+ seconds;
- char session_time[20];
- char uname[6 + NAMEDATALEN];
- char dbname[10 + NAMEDATALEN];
- char remote_host[7 + NI_MAXHOST];
- char remote_port[7 + NI_MAXSERV];
-
- snprintf(uname, sizeof(uname), " user=%s", port->user_name);
- snprintf(dbname, sizeof(dbname), " database=%s", port->database_name);
- snprintf(remote_host, sizeof(remote_host), " host=%s",
- port->remote_host);
- if (port->remote_port[0])
- snprintf(remote_port, sizeof(remote_port), " port=%s", port->remote_port);
- else
- remote_port[0] = '\0';
-
gettimeofday(&end, NULL);
-
if (end.tv_usec < port->session_start.tv_usec)
{
end.tv_sec--;
@@ -3557,28 +3541,16 @@
end.tv_sec -= port->session_start.tv_sec;
end.tv_usec -= port->session_start.tv_usec;
+ /* for stricter accuracy here we could round - this is close enough */
hours = end.tv_sec / SECS_PER_HOUR;
end.tv_sec %= SECS_PER_HOUR;
minutes = end.tv_sec / SECS_PER_MINUTE;
seconds = end.tv_sec % SECS_PER_MINUTE;
- /* if time has gone backwards for some reason say so, or print time */
-
- if (end.tv_sec < 0)
- snprintf(session_time, sizeof(session_time), "negative!");
- else
-
- /*
- * for stricter accuracy here we could round - this is close
- * enough
- */
- snprintf(session_time, sizeof(session_time),
- "%d:%02d:%02d.%02d",
- hours, minutes, seconds, (int) (end.tv_usec / 10000));
-
- ereport(
- LOG,
- (errmsg("disconnection: session time: %s%s%s%s%s",
- session_time, uname, dbname, remote_host, remote_port)));
-
+ ereport(LOG,
+ (errmsg("disconnection: session time=%d:%02d:%02d.%02d,"
+ " user=%s, database=%s, host=%s, %s%s",
+ hours, minutes, seconds, (int) (end.tv_usec / 10000),
+ port->user_name, port->database_name, port->remote_host,
+ port->remote_port[0] ? "port=" : "", port->remote_port)));
}
On Mon, 2005-03-10 at 12:28 -0400, Qingqing Zhou wrote: > Ooops, this wording change is inconsistent with Log_connections, so I > reverted it. Updated patch is attached. Barring any objections, I'll apply this to HEAD tomorrow. -Neil
On Mon, 2005-03-10 at 12:28 -0400, Qingqing Zhou wrote: > Ooops, this wording change is inconsistent with Log_connections, so I > reverted it. Updated patch is attached. Other inconsistencies: - the patch prints a trailing comma when remote->port is the empty string - the patch separates the elements of the ereport() line with commas, whereas the original behavior was to use a single space as a separator - the original behavior was to prefix the session time with "session time:"; the patch changed this to "session time=" The latter two changes aren't necessarily incorrect, but I reverted them anyway, for the sake of consistency with log_connections. A revised patch is attached -- I've applied it to HEAD. -Neil
Attachment
"Neil Conway" <neilc@samurai.com> wrote > > Other inconsistencies: > > - the patch prints a trailing comma when remote->port is the empty > string > > - the patch separates the elements of the ereport() line with commas, > whereas the original behavior was to use a single space as a separator > > - the original behavior was to prefix the session time with "session > time:"; the patch changed this to "session time=" > > The latter two changes aren't necessarily incorrect, but I reverted them > anyway, for the sake of consistency with log_connections. > Yes, you are right, Thanks, Qingqing