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

On Wed, Jan 29, 2014 at 3:11 PM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
> On 14 January 2014 08:38, Christian Kruse <christian(at)2ndquadrant(dot)com> wrote:
>> Hi,
>> On 13/01/14 20:06, Heikki Linnakangas wrote:
>>> On 12/17/2013 04:58 PM, Christian Kruse wrote:
>>> >attached you will find a patch for showing the current transaction id
>>> >(xid) and the xmin of a backend in pg_stat_activty and the xmin in
>>> >pg_stat_replication.
>>>
>>> Docs.
>>
>> Thanks, update with updated docs is attached.
>
> Looks simple enough and useful for working out which people are
> holding up CONCURRENT activities.
>
> I've not been involved with this patch, so any objections to me doing
> final review and commit?

Nope, but I think this patch is broken. It looks to me like it's
conflating the process offset in the BackendStatus array with its
backendId, which does not seem like a good idea even if it happens to
work at present. And the way BackendIdGetProc() is used looks unsafe,
too: the contents might no longer be valid by the time we read them.
I suspect we should have a new accessor function that takes a backend
ID and copies the xid and xmin to pointers provided by the client
while holding the lock. I also note that the docs seem to need some
copy-editing:

+ <entry>The current <xref linked="ddl-system-columns">xmin
value.</xref></entry>

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company