Re: Patch: show relation and tuple infos of a lock to acquire

From: Andres Freund <andres(at)2ndquadrant(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Simon Riggs <simon(at)2ndQuadrant(dot)com>, Christian Kruse <christian(at)2ndQuadrant(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Patch: show relation and tuple infos of a lock to acquire
Date: 2014-01-02 09:02:41
Message-ID: 20140102090241.GD2683@awork2.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2013-12-31 11:36:36 -0500, Tom Lane wrote:
> Simon Riggs <simon(at)2ndQuadrant(dot)com> writes:
> > On 31 December 2013 09:12, Christian Kruse <christian(at)2ndquadrant(dot)com> wrote:
> >> Output with patch:
> >>
> >> LOG: process 24774 acquired ShareLock on transaction 696 after 11688.720 ms
> >> CONTEXT: relation name: foo (OID 16385)
> >> tuple (ctid (0,1)): (1)
>
> > That is useful info.
>
> > I think the message should be changed to say this only, without a context line
>
> > LOG: process 24774 acquired ShareLock on relation "foo" (OID 16385)
> > tuple (0,1) after 11688.720 ms
>
> > My reason is that pid 24774 was waiting for a *tuple lock* and it was
> > eventually granted, so thats what it should say.
>
> No, that wasn't what it was waiting for, and lying to the user like that
> isn't going to make things better.

Agreed.

> Christian's idea of a context line seems plausible to me. I don't
> care for this implementation too much --- a global variable? Ick.

Yea, the data should be stored in ErrorContextCallback.arg instead.

> Make a wrapper function for XactLockTableWait instead, please.

I don't think that'd work out all too nicely - we do the
XactLockTableWait() inside other functions like MultiXactIdWait(),
passing all the detail along for those would end up being ugly. So using
error context callbacks properly seems like the best way in the end.

> And I'm not real sure that showing the whole tuple contents is a good
> thing; I can see people calling that a security leak, not to mention
> that the performance consequences could be dire.

I don't think that the security argument has too much merit given
today's PG - we already log the full tuple for various constraint
violations. And we also accept the performance penalty there. I don't
see any easy way to select a sensible subset of columns to print here,
and printing the columns is what would make investigating issues around
this so much easier. At the very least we'd need to print the pkey
columns and the columns of the unique key that might have been violated.

Greetings,

Andres Freund

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2014-01-02 09:10:14 Re: What exactly is drop-index-concurrently-1.spec trying to test?
Previous Message Andres Freund 2014-01-02 08:56:30 Re: Patch: show relation and tuple infos of a lock to acquire