Hi,
On 19/03/14 15:12, Alvaro Herrera wrote:
> I hope the silence meant assent, because I have pushed this patch now.
Great, thanks!
Best regards,
--
Christian Kruse http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
pgpYfuVjR_gg_.pgp
Descr
Alvaro Herrera escribió:
> Tom Lane escribió:
> > I think the enum idea you floated is probably worth doing. It's
> > certainly no more complex than passing a string, no?
>
> Okay, done that way, attached. I think this one solves all concerns
> there were.
I hope the silence meant assent, beca
Tom Lane escribió:
> Alvaro Herrera writes:
> > Please see my reply to Robert. My proposal (in form of a patch) is
> > while operating on tuple (0,2) in table "foo": updating tuple
> > Would this work for you?
>
> It's pretty lousy from a readability standpoint, even in English;
> I shudder to
On Tue, Mar 18, 2014 at 12:53 PM, Tom Lane wrote:
> Alvaro Herrera writes:
>> Please see my reply to Robert. My proposal (in form of a patch) is
>> while operating on tuple (0,2) in table "foo": updating tuple
>> Would this work for you?
>
> It's pretty lousy from a readability standpoint, eve
Alvaro Herrera writes:
> Please see my reply to Robert. My proposal (in form of a patch) is
> while operating on tuple (0,2) in table "foo": updating tuple
> Would this work for you?
It's pretty lousy from a readability standpoint, even in English;
I shudder to think what it might come out as
Tom Lane escribió:
> Robert Haas writes:
> > Well, if we're back to one version of the message, and I'm glad we
> > are, can we go back to saying:
>
> > CONTEXT: while updating tuple (0,2) in relation "public"."foo" of
> > database "postgres"
>
> If I end up being the one who commits this, it's
Robert Haas writes:
> Well, if we're back to one version of the message, and I'm glad we
> are, can we go back to saying:
> CONTEXT: while updating tuple (0,2) in relation "public"."foo" of
> database "postgres"
If I end up being the one who commits this, it's going to say
while updating tuple
Robert Haas escribió:
> On Tue, Mar 18, 2014 at 12:00 AM, Amit Kapila wrote:
> >> Therefore I think the only case worth considering here is when
> >> database/relation/TID are all defined. The other cases where there is
> >> partial information are dead code; and the case where there is nothing
>
On Tue, Mar 18, 2014 at 12:00 AM, Amit Kapila wrote:
>> Therefore I think the only case worth considering here is when
>> database/relation/TID are all defined. The other cases where there is
>> partial information are dead code; and the case where there is nothing
>> defined (such as the one in
On Tue, Mar 18, 2014 at 11:59 AM, Greg Stark wrote:
> On Tue, Mar 18, 2014 at 3:42 AM, Amit Kapila wrote:
>> The message for exclusive lock on tuple print the database information.
>
> It is true that it is possible to have a deadlock or lock chains that
> involves locks on other databases.
> In
On Tue, Mar 18, 2014 at 3:42 AM, Amit Kapila wrote:
> The message for exclusive lock on tuple print the database information.
It is true that it is possible to have a deadlock or lock chains that
involves locks on other databases.
In this example the table "test" is not in the database that just
On Tue, Mar 18, 2014 at 3:38 AM, Alvaro Herrera
wrote:
> Here's an adjusted version. In this one, the extra info is not used to
> construct a string from pieces, but instead it puts it at the end, like
> this:
>
> LOG: process 18899 still waiting for ShareLock on transaction 697 after
> 1000.20
On Tue, Mar 18, 2014 at 5:51 AM, Tom Lane wrote:
> Alvaro Herrera writes:
>> 1. MyProcPort contains the database name; no need for the
>> get_database_name() call in there.
>
> Wait. A. Minute. This patch wants to print the current database name in
> the message? What on earth for? What other
Tom Lane escribió:
> Alvaro Herrera writes:
> > 1. MyProcPort contains the database name; no need for the
> > get_database_name() call in there.
>
> Wait. A. Minute. This patch wants to print the current database name in
> the message? What on earth for? What other error messages do you see
>
On 2014-03-17 20:21:18 -0400, Tom Lane wrote:
> Alvaro Herrera writes:
> > 1. MyProcPort contains the database name; no need for the
> > get_database_name() call in there.
>
> Wait. A. Minute. This patch wants to print the current database name in
> the message? What on earth for? What other e
Alvaro Herrera writes:
> 1. MyProcPort contains the database name; no need for the
> get_database_name() call in there.
Wait. A. Minute. This patch wants to print the current database name in
the message? What on earth for? What other error messages do you see
doing that?
It should print a ta
Alvaro Herrera escribió:
> Another thing that jumped at me is that passing a TID but not a Relation
> is fairly useless as it stands. We might try to add some more stuff
> later, such as printing tuple contents as previous versions of the patch
> did, but given the opposition the idea had previou
Here's an adjusted version. In this one, the extra info is not used to
construct a string from pieces, but instead it puts it at the end, like
this:
LOG: process 18899 still waiting for ShareLock on transaction 697 after
1000.203 ms
CONTEXT: while operating on tuple (0,2) in relation "public".
On Mon, Mar 17, 2014 at 4:20 PM, Christian Kruse
wrote:
> Hi Amit,
>
> I've been ill the last few days, so sorry for my late response.
Sorry to hear and no problem for delay.
> I don't think that this fixes the translation guideline issues brought
> up by Robert. This produces differing strin
Hi Amit,
I've been ill the last few days, so sorry for my late response.
> I have updated the patch to pass TID and operation information in
> error context and changed some of the comments in code.
> Let me know if the added operation information is useful, else
> we can use better generic messa
On Thu, Mar 13, 2014 at 8:06 PM, Amit Kapila wrote:
> On Thu, Mar 13, 2014 at 7:10 PM, Robert Haas wrote:
>> On Thu, Mar 13, 2014 at 12:45 AM, Amit Kapila
>> wrote:
>>> _bt_doinsert - "insert index tuple (X,Y)" (here it will refer to index tuple
>>> location)
>>
>> I don't think that giving the
In this loop,
> + for (i = 0; i < desc->natts; i++)
> + {
> + char *val;
> + int vallen;
> +
> + vallen = strlen(val);
> +
On Thu, Mar 13, 2014 at 7:10 PM, Robert Haas wrote:
> On Thu, Mar 13, 2014 at 12:45 AM, Amit Kapila wrote:
>> _bt_doinsert - "insert index tuple (X,Y)" (here it will refer to index tuple
>> location)
>
> I don't think that giving the index tuple location is going to be very
> helpful; can we get
Robert Haas writes:
> Well, it's sounding like we can only display the whole tuple if (1)
> the message level is less than ERROR and (2) the snapshot is an MVCC
> snapshot. That's an annoying and hard-to-document set of limitations.
> But we should be able to display the TID always, so I think w
On Thu, Mar 13, 2014 at 12:45 AM, Amit Kapila wrote:
>> While attempting to "operate in"? That seems like unhelpful
>> weasel-wording. I wonder if we ought to have separate messages for
>> each possibility, like "delete tuple (X,Y)" when called from
>> heap_delete(), "update tuple (X,Y)", "check
On Thu, Mar 13, 2014 at 12:16 AM, Robert Haas wrote:
> On Tue, Mar 11, 2014 at 3:53 AM, Amit Kapila wrote:
>> Places where tuple info not available
>>
>> LOG: process 5788 still waiting for ShareLock on transaction 679 after
>> 1014.000
>> ms
>> CONTEXT: while attempting to operate in relatio
On Tue, Mar 11, 2014 at 3:53 AM, Amit Kapila wrote:
> Places where tuple info not available
>
> LOG: process 5788 still waiting for ShareLock on transaction 679 after
> 1014.000
> ms
> CONTEXT: while attempting to operate in relation "public"."idx_t1" of
> database
> "postgres"
The way the c
On Tue, Mar 11, 2014 at 2:32 PM, Christian Kruse
wrote:
> On 11/03/14 13:23, Amit Kapila wrote:
>> Could you please once check (if you are comfortable doing so) wherever
>> this patch is passing tuple, whether it is okay to pass it based on
>> visibility
>> rules, else I will again verify it once
Hi,
On 11/03/14 13:23, Amit Kapila wrote:
> [… snip …]
> So I think it's better to leave logging it as you have done in
> patch.
Agreed.
> […]
> 2. Name new functions as
> MultiXactIdWaitExtended()/XactLockTableWaitExtended()
> or MultiXactIdWaitEx()/XactLockTableWaitEx().
> You can find some
On Mon, Mar 10, 2014 at 5:14 PM, Christian Kruse
wrote:
> On 09/03/14 12:15, Amit Kapila wrote:
>> [...] due to which the message it displays seems to be
>> incomplete. Message it displays is as below:
>>
>> LOG: process 1800 still waiting for ShareLock on transaction 679 after
>> 1014.000
>> m
Hi,
On 10/03/14 14:59, Robert Haas wrote:
> On Mon, Mar 10, 2014 at 7:44 AM, Christian Kruse
> wrote:
> > [ response to review ]
>
> This response seems to have made no mention of point #7 from Amit's
> review, which seems to me to be a rather important one.
Just didn't notice it because the pr
On Mon, Mar 10, 2014 at 7:44 AM, Christian Kruse
wrote:
> [ response to review ]
This response seems to have made no mention of point #7 from Amit's
review, which seems to me to be a rather important one.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Hi,
thanks for the continued review.
On 09/03/14 12:15, Amit Kapila wrote:
> 1.
> "> ISTM that you should be printing just the value and the unique index
> > there, and not any information about the tuple proper.
>
> Good point, I will have a look at this."
>
> This point is still not handled i
On Thu, Feb 27, 2014 at 4:14 PM, Christian Kruse
wrote:
> On 25/02/14 16:11, Robert Haas wrote:
>> Reading this over, I'm not sure I understand why this is a CONTEXT at
>> all and not just a DETAIL for the particular error message that it's
>> supposed to be decorating. Generally CONTEXT should b
On Tue, Mar 4, 2014 at 2:18 PM, Simon Riggs wrote:
> On 4 March 2014 04:18, Amit Kapila wrote:
>> I know that patch truncates the values if they are greater than certain
>> length (30), but the point is why it is not sufficient to have tuple location
>> (and primary key if available) which unique
On 4 March 2014 04:18, Amit Kapila wrote:
> On Mon, Mar 3, 2014 at 3:38 PM, Andres Freund wrote:
>> On 2014-02-28 20:55:20 +0530, Amit Kapila wrote:
>>> On Thu, Feb 27, 2014 at 4:14 PM, Christian Kruse
>>> > Well, as I already stated: we don't. I copied the behavior we use in
>>> > CHECK constrai
On Mon, Mar 3, 2014 at 3:38 PM, Andres Freund wrote:
> On 2014-02-28 20:55:20 +0530, Amit Kapila wrote:
>> On Thu, Feb 27, 2014 at 4:14 PM, Christian Kruse
>> > Well, as I already stated: we don't. I copied the behavior we use in
>> > CHECK constraints (ExecBuildSlotValueDescription()).
>>
>> I th
On Mon, Mar 3, 2014 at 3:46 PM, Andres Freund wrote:
> On 2014-03-01 13:29:18 +0530, Amit Kapila wrote:
>> With new patch, the message while updating locked rows will be displayed
>> as below:
>>
>> LOG: process 364 still waiting for ShareLock on transaction 678 after
>> 1014.000ms
>> CONTEXT: w
On 2014-03-01 13:29:18 +0530, Amit Kapila wrote:
> With new patch, the message while updating locked rows will be displayed
> as below:
>
> LOG: process 364 still waiting for ShareLock on transaction 678 after
> 1014.000ms
> CONTEXT: while attempting to lock tuple (0,2) with values (2) in relati
On 2014-02-28 20:55:20 +0530, Amit Kapila wrote:
> On Thu, Feb 27, 2014 at 4:14 PM, Christian Kruse
> >> I wouldn't be inclined to dump the whole tuple under any
> >> circumstances. That could be a lot more data than what you want
> >> dumped in your log. The PK could already be somewhat unreason
On Thu, Feb 27, 2014 at 4:14 PM, Christian Kruse
wrote:
> On 25/02/14 16:11, Robert Haas wrote:
>> Reading this over, I'm not sure I understand why this is a CONTEXT at
>> all and not just a DETAIL for the particular error message that it's
>> supposed to be decorating. Generally CONTEXT should b
On Thu, Feb 27, 2014 at 4:14 PM, Christian Kruse
wrote:
> Hi,
>
> On 25/02/14 16:11, Robert Haas wrote:
>> On Mon, Feb 24, 2014 at 10:13 AM, Christian Kruse
>> wrote:
>> > To be honest, I don't like the idea of setting up this error context
>> > only for log_lock_wait messages. This sounds unnece
Hi,
On 25/02/14 16:11, Robert Haas wrote:
> On Mon, Feb 24, 2014 at 10:13 AM, Christian Kruse
> wrote:
> > To be honest, I don't like the idea of setting up this error context
> > only for log_lock_wait messages. This sounds unnecessary complex to me
> > and I think that in the few cases where th
On Mon, Feb 24, 2014 at 10:13 AM, Christian Kruse
wrote:
> To be honest, I don't like the idea of setting up this error context
> only for log_lock_wait messages. This sounds unnecessary complex to me
> and I think that in the few cases where this message doesn't add a
> value (and thus is useless
Hi,
attached you will find a new version of the patch, removing the ctid
text but leaving the ctid itself in the message.
On 23/02/14 11:14, Amit Kapila wrote:
> >> In general, why I am suggesting to restrict display of newly added
> >> context for the case it is added to ensure that it doesn't g
On Sat, Feb 22, 2014 at 3:17 PM, Andres Freund wrote:
> On 2014-02-22 11:53:24 +0530, Amit Kapila wrote:
>
>> In general, why I am suggesting to restrict display of newly added
>> context for the case it is added to ensure that it doesn't get started
>> displaying in other cases where it might mak
On 2014-02-22 11:53:24 +0530, Amit Kapila wrote:
> On Fri, Feb 21, 2014 at 4:55 PM, Christian Kruse
> wrote:
> > Hi,
> >> 1. New context added by this patch is display at wrong place
> >> [...]
> >> Do this patch expect to print the context at cancel request
> >> as well?
> >
> > To be honest, I d
On Fri, Feb 21, 2014 at 4:55 PM, Christian Kruse
wrote:
> Hi,
>> 1. New context added by this patch is display at wrong place
>> [...]
>> Do this patch expect to print the context at cancel request
>> as well?
>
> To be honest, I don't see a problem here. If you cancel the request in
> most cases
Hi,
On 19.02.2014 08:11, Amit Kapila wrote:
> I have looked into this patch. Below are some points which I
> think should be improved in this patch:
Thank you for your review.
> 1. New context added by this patch is display at wrong place
> […]
> Do this patch expect to print the context at canc
On Thu, Jan 2, 2014 at 4:38 PM, Christian Kruse
wrote:
> Hi,
>
> On 02/01/14 10:02, Andres Freund wrote:
>> > 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 ErrorCont
Hi,
On 02/01/14 10:02, Andres Freund wrote:
> > 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.
Fixed.
I also palloc() the ErrorConte
On 2014-01-02 01:40:38 -0800, Peter Geoghegan wrote:
> On Thu, Jan 2, 2014 at 12:56 AM, Andres Freund wrote:
> > I agree that the message needs improvement, but I don't agree that we
> > shouldn't lock the tuple's location. If you manually investigate the
> > situation that's where you'll find the
On Thu, Jan 2, 2014 at 12:56 AM, Andres Freund wrote:
> I agree that the message needs improvement, but I don't agree that we
> shouldn't lock the tuple's location. If you manually investigate the
> situation that's where you'll find the conflicting tuple - I don't see
> what we gain by not loggin
On 2013-12-31 11:36:36 -0500, Tom Lane wrote:
> Simon Riggs writes:
> > On 31 December 2013 09:12, Christian Kruse
> > wrote:
> >> Output with patch:
> >>
> >> LOG: process 24774 acquired ShareLock on transaction 696 after 11688.720
> >> ms
> >> CONTEXT: relation name: foo (OID 16385)
> >> t
On 2013-12-31 13:56:53 -0800, Peter Geoghegan wrote:
> ISTM that you should be printing just the value and the unique index
> there, and not any information about the tuple proper. Doing any less
> could be highly confusing.
I agree that the message needs improvement, but I don't agree that we
sho
On 31 December 2013 17:06, Simon Riggs wrote:
> On 31 December 2013 16:36, Tom Lane wrote:
>> Simon Riggs writes:
>>> On 31 December 2013 09:12, Christian Kruse
>>> wrote:
Output with patch:
LOG: process 24774 acquired ShareLock on transaction 696 after 11688.720
ms
On 31/12/13 11:36, Tom Lane wrote:
> Christian's idea of a context line seems plausible to me. I don't
> care for this implementation too much --- a global variable? Ick.
> Make a wrapper function for XactLockTableWait instead, please.
Point taken. You are right.
> And I'm not real sure that sh
Hi,
On 31/12/13 13:56, Peter Geoghegan wrote:
> I think that this is a worthwhile effort, but like Tom I don't think
> that it's universally true that these waiters are waiting on a tuple
> lock. Most obviously, the XactLockTableWait() call you've modified
> within nbtinsert.c is not.
This is why
On Tue, Dec 31, 2013 at 1:12 AM, Christian Kruse
wrote:
> I created a patch improving the log_lock_wait messages by adding
> relation infos (name and OID) as well as tuple infos (if present –
> ctid and, if available, the tuple itself) in the context.
I think that this is a worthwhile effort, but
On 31 December 2013 16:36, Tom Lane wrote:
> Simon Riggs writes:
>> On 31 December 2013 09:12, Christian Kruse 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)): (
Simon Riggs writes:
> On 31 December 2013 09:12, Christian Kruse 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 s
On 31 December 2013 09:12, Christian Kruse wrote:
> Output w/o patch:
>
> LOG: process 24774 acquired ShareLock on transaction 696 after 11688.720 ms
>
> Output with patch:
>
> LOG: process 24774 acquired ShareLock on transaction 696 after 11688.720 ms
> CONTEXT: relation name: foo (OID 16385)
Hi there,
I created a patch improving the log_lock_wait messages by adding
relation infos (name and OID) as well as tuple infos (if present –
ctid and, if available, the tuple itself) in the context.
Sample output (log_lock_waits=on required):
session 1:
CREATE TABLE foo (val integer);
INSERT IN
63 matches
Mail list logo
