Re: Patch: show xid and xmin in pg_stat_activity and pg_stat_replication

From: Christian Kruse <christian(at)2ndquadrant(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, Andres Freund <andres(at)2ndquadrant(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Simon Riggs <simon(at)2ndquadrant(dot)com>, Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>
Subject: Re: Patch: show xid and xmin in pg_stat_activity and pg_stat_replication
Date: 2014-02-17 09:44:41
Message-ID: 5301DA09.1030701@2ndquadrant.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Robert,

Am 15.02.14 05:03, schrieb Robert Haas:
> Well, this version of the patch reveals a mighty interesting point: a
> lot of the people who are calling pgstat_fetch_stat_beentry() don't
> need this additional information and might prefer not to pay the cost
> of fetching it.

Well, the cost is already paid due to the fact that this patch uses
LocalPgBackendStatus instead of PgBackendStatus in
pgstat_read_current_status(). And pgstat_fetch_stat_beentry() returns
a pointer instead of a copy, so the cost is rather small, too.

> None of pg_stat_get_backend_pid,
> pg_stat_get_backend_dbid, pg_stat_get_backend_userid,
> pg_stat_get_backend_activity, pg_stat_get_backend_activity,
> pg_stat_get_backend_waiting, pg_stat_get_backend_activity_start,
> pg_stat_get_backend_xact_start, pg_stat_get_backend_start,
> pg_stat_get_backend_client_addr, pg_stat_get_backend_client_port,
> pg_stat_get_backend_client_port, and pg_stat_get_db_numbackends
> actually need this new information; it's only ever used in one place.
> So it seems like it might be wise to have pgstat_fetch_stat_beentry
> continue to return the PgBackendStatus * and add a new function
> pgstat_fetch_stat_local_beentry to fetch the LocalPgBackendStatus *;
> then most of these call sites wouldn't need to change.

This is true for now. But one of the purposes of using
LocalPgBackendStatus instead of PgBackendStatus was to be able to add
more fields like this in future. And thus we might need to change this
in future, so why not do it now?

And I also agree to Andres.

> It would still be the case that pgstat_read_current_status() pays the
> price of fetching this information even if pg_stat_get_activity is
> never called. But since that's probably by far the most commonly-used
> API for this information, that's probably OK.

I agree.

I will change it if this is really wanted, but I think it would be a
good idea to do it this way.

Best regards,

--
Christian Kruse http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Sergey Muraviov 2014-02-17 10:05:38 Re: Problem with displaying "wide" tables in psql
Previous Message Bjorn Munch 2014-02-17 09:38:29 Re: Ctrl+C from sh can shut down daemonized PostgreSQL cluster