implement query_start for pg_stat_activity

Neil Conway <neilc@samurai.com> writes:

(1) The natural name for the backend function is
pg_stat_get_backend_activity_start(), which at 34 exceeds the
NAMEDATALEN limitation in previous releases of PostgreSQL. While that
limit has been raised, it seems a shame to me to not allow users to
manual lower it again -- so I renamed the function to
pg_stat_get_backend_qry_start(), and renamed the existing function that
fetches the query string from pg_stat_get_backend_activity() to
pg_stat_get_backend_qry() for consistency. If someone thinks that's the
wrong decision, let me know.

NAMEDATALEN=32 will have been history for two releases when this gets
out. I don't agree with artificially constricting a function name to
conform to an obsolete restriction --- and for *sure* I don't agree with
renaming an existing function to make it line up with a new, artificial
name ...

(2) I wasn't sure how to convert a struct timeval into a PostgreSQL
timestamp type, so I hacked something together involving ctime() and
timestamp_in(), but it seems clearly wrong.

GetCurrentAbsoluteTimeUsec() followed by now() seem to do it at the
moment ... but they do look pretty historically encumbered themselves.
Care to offer a proposal for simplifying this code?

regards, tom lane

On Wed, 2003-02-19 at 01:04, Tom Lane wrote:

NAMEDATALEN=32 will have been history for two releases when this gets
out. I don't agree with artificially constricting a function name to
conform to an obsolete restriction --- and for *sure* I don't agree with
renaming an existing function to make it line up with a new, artificial
name ...

Fair enough.

(2) I wasn't sure how to convert a struct timeval into a PostgreSQL
timestamp type, so I hacked something together involving ctime() and
timestamp_in(), but it seems clearly wrong.

GetCurrentAbsoluteTimeUsec() followed by now() seem to do it at the
moment ...

Can you elaborate a bit? Since now() returns the time the current txn
started, that doesn't seem to be what I'd need -- and isn't
GetCurrentAbsoluteTimeUsec() essentially just an (ugly) wrapper over
gettimeofday()?

I think a similar function to timeofday() that returns a timestamp would
probably be ideal... (which reminds me, we should probably think about
how to replace timeofday() with something that doesn't return a string).

Care to offer a proposal for simplifying this code?

To be honest, I wouldn't know where to start -- I don't really know much
about the intricacies of datetime manipulation...

Cheers,

Neil
--
Neil Conway <neilc@samurai.com> || PGP Key ID: DB3C29FC

Neil Conway <neilc@samurai.com> writes:

Care to offer a proposal for simplifying this code?

To be honest, I wouldn't know where to start -- I don't really know much
about the intricacies of datetime manipulation...

Me either. Tom Lockhart was our datetime-meister, but he's gafiated.
We need someone new to take charge of that code... there's definitely
work remaining to be done there...

regards, tom lane

On Wed, 2003-02-19 at 00:27, Neil Conway wrote:

This patch implements the TODO item

- Add start time to pg_stat_activity

I've attached a revised version of the patch with the following changes:

- use longer function names, as suggested by Tom

- (unrelated) replace some instances where snprintf() was called with a
constant buffer size with sizeof

- documentation improvements

- update regression tests

- change the method of finding the current date & time -- it's still an
ugly hack, although arguably less so. Now we do:

AbsoluteTime sec;
int usec;

sec = GetCurrentAbsoluteTimeUsec(&usec);

/* when displaying the time */
Timestamp result;

#ifdef HAVE_INT64_TIMESTAMP
result = (((sec - ((date2j(2000, 1, 1) - date2j(1970, 1, 1)) * 86400))
* INT64CONST(1000000)) + usec);
#else
result = (sec + (usec * 1.0e-6) - ((date2j(2000, 1, 1) -
date2j(1970, 1, 1)) * 86400));
#endif

which is the same method now() uses internally.

Cheers,

Neil
--
Neil Conway <neilc@samurai.com> || PGP Key ID: DB3C29FC

Neil Conway <neilc@samurai.com> writes:

- change the method of finding the current date & time -- it's still an
ugly hack, although arguably less so. Now we do:

Can't we push that into some subroutine exported by one of the
datetime-related utils/adt files? It seems a bad idea for pgstatfuncs.c
to get so cozy with the internal representation of some other module's
datatype.

regards, tom lane

On Wed, 2003-02-19 at 01:45, Neil Conway wrote:

On Wed, 2003-02-19 at 01:04, Tom Lane wrote:

NAMEDATALEN=32 will have been history for two releases when this gets
out. I don't agree with artificially constricting a function name to
conform to an obsolete restriction --- and for *sure* I don't agree with
renaming an existing function to make it line up with a new, artificial
name ...

Fair enough.

(2) I wasn't sure how to convert a struct timeval into a PostgreSQL
timestamp type, so I hacked something together involving ctime() and
timestamp_in(), but it seems clearly wrong.

GetCurrentAbsoluteTimeUsec() followed by now() seem to do it at the
moment ...

Can you elaborate a bit? Since now() returns the time the current txn
started, that doesn't seem to be what I'd need -- and isn't
GetCurrentAbsoluteTimeUsec() essentially just an (ugly) wrapper over
gettimeofday()?

I think a similar function to timeofday() that returns a timestamp would
probably be ideal... (which reminds me, we should probably think about
how to replace timeofday() with something that doesn't return a string).

Care to offer a proposal for simplifying this code?

To be honest, I wouldn't know where to start -- I don't really know much
about the intricacies of datetime manipulation...

I did a quick patch a while ago that would simply use Abstime -- setting
query_start and connection_start (when the backend was fired up).

Another thing it did was to change pg_stat_activity into a single SRF
removing a good chunk of the other junk.

Anyway, after using it for a while on a test system. I removed the Usec
precision as short queries disappear quickly enough for this not to be
interesting nor was there much point in sending the time from the
backend to the stat collector, as even a 5 second delay in receiving the
UDP packet didn't make any difference in the information I was looking
for -- enough information to make an snmp event about a rogue query or
connection.

Anyway, attached is what I've been using for curiosity sakes. Updated
for -TIP and it compiles but I didn't test to see if it still works.

--
Rod Taylor <rbt@rbt.ca>

PGP Key: http://www.rbt.ca/rbtpub.asc

Your patch has been added to the PostgreSQL unapplied patches list at:

http://momjian.postgresql.org/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

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

Neil Conway wrote:

On Wed, 2003-02-19 at 00:27, Neil Conway wrote:

This patch implements the TODO item

- Add start time to pg_stat_activity

I've attached a revised version of the patch with the following changes:

- use longer function names, as suggested by Tom

- (unrelated) replace some instances where snprintf() was called with a
constant buffer size with sizeof

- documentation improvements

- update regression tests

- change the method of finding the current date & time -- it's still an
ugly hack, although arguably less so. Now we do:

AbsoluteTime sec;
int usec;

sec = GetCurrentAbsoluteTimeUsec(&usec);

/* when displaying the time */
Timestamp result;

#ifdef HAVE_INT64_TIMESTAMP
result = (((sec - ((date2j(2000, 1, 1) - date2j(1970, 1, 1)) * 86400))
* INT64CONST(1000000)) + usec);
#else
result = (sec + (usec * 1.0e-6) - ((date2j(2000, 1, 1) -
date2j(1970, 1, 1)) * 86400));
#endif

which is the same method now() uses internally.

Cheers,

Neil
--
Neil Conway <neilc@samurai.com> || PGP Key ID: DB3C29FC

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 1: subscribe and unsubscribe commands go to majordomo@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

Patch applied. Thanks.

Initdb forced.

New output:

test=> select pg_stat_activity.*;
datid | datname | procpid | usesysid | usename | current_query | query_start
-------+---------+---------+----------+----------+---------------+----------------------------
17054 | test | 1147 | 1 | postgres | <IDLE> | 2003-03-20 03:01:51.432215
(1 row)

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

Neil Conway wrote:

On Wed, 2003-02-19 at 00:27, Neil Conway wrote:

This patch implements the TODO item

- Add start time to pg_stat_activity

I've attached a revised version of the patch with the following changes:

- use longer function names, as suggested by Tom

- (unrelated) replace some instances where snprintf() was called with a
constant buffer size with sizeof

- documentation improvements

- update regression tests

- change the method of finding the current date & time -- it's still an
ugly hack, although arguably less so. Now we do:

AbsoluteTime sec;
int usec;

sec = GetCurrentAbsoluteTimeUsec(&usec);

/* when displaying the time */
Timestamp result;

#ifdef HAVE_INT64_TIMESTAMP
result = (((sec - ((date2j(2000, 1, 1) - date2j(1970, 1, 1)) * 86400))
* INT64CONST(1000000)) + usec);
#else
result = (sec + (usec * 1.0e-6) - ((date2j(2000, 1, 1) -
date2j(1970, 1, 1)) * 86400));
#endif

which is the same method now() uses internally.

Cheers,

Neil
--
Neil Conway <neilc@samurai.com> || PGP Key ID: DB3C29FC

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 1: subscribe and unsubscribe commands go to majordomo@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

Also, TODO item now marked as done!

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

Neil Conway wrote:

On Wed, 2003-02-19 at 00:27, Neil Conway wrote:

This patch implements the TODO item

- Add start time to pg_stat_activity

I've attached a revised version of the patch with the following changes:

- use longer function names, as suggested by Tom

- (unrelated) replace some instances where snprintf() was called with a
constant buffer size with sizeof

- documentation improvements

- update regression tests

- change the method of finding the current date & time -- it's still an
ugly hack, although arguably less so. Now we do:

AbsoluteTime sec;
int usec;

sec = GetCurrentAbsoluteTimeUsec(&usec);

/* when displaying the time */
Timestamp result;

#ifdef HAVE_INT64_TIMESTAMP
result = (((sec - ((date2j(2000, 1, 1) - date2j(1970, 1, 1)) * 86400))
* INT64CONST(1000000)) + usec);
#else
result = (sec + (usec * 1.0e-6) - ((date2j(2000, 1, 1) -
date2j(1970, 1, 1)) * 86400));
#endif

which is the same method now() uses internally.

Cheers,

Neil
--
Neil Conway <neilc@samurai.com> || PGP Key ID: DB3C29FC

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 1: subscribe and unsubscribe commands go to majordomo@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