headers
Jeff Janes | 15 Jun 2012 23:51
Picon

Re: Resource Owner reassign Locks

On Mon, Jun 11, 2012 at 9:30 PM, Amit Kapila <amit.kapila <at> huawei.com> wrote:
>> Yes, that means the list has over-flowed.  Once it is over-flowed, it
>> is now invalid for the reminder of the life of the resource owner.

> Don't we need any logic to clear the reference of locallock in owner->locks
> array.

I don't think so.  C doesn't ref count its pointers.

> MAX_RESOWNER_LOCKS - How did you arrive at number 10 for it. Is there any
> specific reason for 10.

I instrumented the code to record the maximum number of locks held by
a resource owner, and report the max when it was destroyed.  (That
code is not in this patch).  During a large pg_dump, the vast majority
of the resource  owners had maximum locks of 2, with some more at 4
and 6.    Then there was one resource owner, for the top-level
transaction, at tens or hundreds of thousands (basically one for every
lockable object).  There was little between 6 and this top-level
number, so I thought 10 was a good compromise, safely above 6 but not
so large that searching through the list itself was likely to bog
down.

Also, Tom independently suggested the same number.

>> Should it emit a FATAL rather than an ERROR?  I thought ERROR was
>> sufficient to make the backend quit, as it is not clear how it could
>> meaningfully recover.
>
> I am not able to visualize any valid scenario in which it can happen unless
(Continue reading)

Permalink | Reply |
headers
Tom Lane | 16 Jun 2012 00:29
Picon

Re: Resource Owner reassign Locks

Jeff Janes <jeff.janes <at> gmail.com> writes:
> On Mon, Jun 11, 2012 at 9:30 PM, Amit Kapila <amit.kapila <at> huawei.com> wrote:
>> MAX_RESOWNER_LOCKS - How did you arrive at number 10 for it. Is there any
>> specific reason for 10.

> I instrumented the code to record the maximum number of locks held by
> a resource owner, and report the max when it was destroyed.  (That
> code is not in this patch).  During a large pg_dump, the vast majority
> of the resource  owners had maximum locks of 2, with some more at 4
> and 6.    Then there was one resource owner, for the top-level
> transaction, at tens or hundreds of thousands (basically one for every
> lockable object).  There was little between 6 and this top-level
> number, so I thought 10 was a good compromise, safely above 6 but not
> so large that searching through the list itself was likely to bog
> down.

> Also, Tom independently suggested the same number.

FYI, I had likewise suggested 10 on the basis of examining pg_dump's
behavior.  It might be a good idea to examine a few other use-cases
before settling on a value.

			regards, tom lane

--

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers <at> postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
(Continue reading)

Permalink | Reply |
headers
Jeff Janes | 16 Jun 2012 04:07
Picon

Re: Resource Owner reassign Locks

On Fri, Jun 15, 2012 at 3:29 PM, Tom Lane <tgl <at> sss.pgh.pa.us> wrote:
> Jeff Janes <jeff.janes <at> gmail.com> writes:
>> On Mon, Jun 11, 2012 at 9:30 PM, Amit Kapila <amit.kapila <at> huawei.com> wrote:
>>> MAX_RESOWNER_LOCKS - How did you arrive at number 10 for it. Is there any
>>> specific reason for 10.
>
>> I instrumented the code to record the maximum number of locks held by
>> a resource owner, and report the max when it was destroyed.  (That
>> code is not in this patch).  During a large pg_dump, the vast majority
>> of the resource  owners had maximum locks of 2, with some more at 4
>> and 6.    Then there was one resource owner, for the top-level
>> transaction, at tens or hundreds of thousands (basically one for every
>> lockable object).  There was little between 6 and this top-level
>> number, so I thought 10 was a good compromise, safely above 6 but not
>> so large that searching through the list itself was likely to bog
>> down.
>
>> Also, Tom independently suggested the same number.
>
> FYI, I had likewise suggested 10 on the basis of examining pg_dump's
> behavior.  It might be a good idea to examine a few other use-cases
> before settling on a value.

Looking at the logging output of a "make check" run, there are many
cases where the list would have overflown (max locks was >10), but in
all of them the number of locks held at the time of destruction was
equal to, or only slightly less than, the size of the local lock hash
table.  So iterating over a large memorized list would not save much
computational complexity over iterating over the entire hash table
(although the constant factor in iterating over pointers in an array
(Continue reading)

Permalink | Reply |
headers
Amit kapila | 16 Jun 2012 08:04
Favicon

Re: Resource Owner reassign Locks

> I don't think so.  C doesn't ref count its pointers.
You are right I have misunderstood.

> I don't think that lock tags have good human readable formats, and just
> a pointer dump probably wouldn't be much use when something that can
> never happen has happened.  But I'll at least add a reference to the
> resource owner if this stays in.

I have checked in lock.c file for the message where lock tags have been used.
elog(ERROR, "lock %s on object %u/%u/%u is already held",
    lockMethodTable->lockModeNames[lockmode],
    lock->tag.locktag_field1, lock->tag.locktag_field2,
    lock->tag.locktag_field3);

This can give more information about erroneous lock.

________________________________________
From: Jeff Janes [jeff.janes <at> gmail.com]
Sent: Saturday, June 16, 2012 3:21 AM
To: Amit kapila
Cc: pgsql-hackers
Subject: Re: [HACKERS] Resource Owner reassign Locks

On Mon, Jun 11, 2012 at 9:30 PM, Amit Kapila <amit.kapila <at> huawei.com> wrote:
>> Yes, that means the list has over-flowed.  Once it is over-flowed, it
>> is now invalid for the reminder of the life of the resource owner.

> Don't we need any logic to clear the reference of locallock in owner->locks
> array.
(Continue reading)

Permalink | Reply |
headers
Heikki Linnakangas | 18 Jun 2012 12:54
Favicon

Re: Resource Owner reassign Locks

On 16.06.2012 09:04, Amit kapila wrote:
>> I don't think so.  C doesn't ref count its pointers.
> You are right I have misunderstood.
>
>> I don't think that lock tags have good human readable formats, and just
>> a pointer dump probably wouldn't be much use when something that can
>> never happen has happened.  But I'll at least add a reference to the
>> resource owner if this stays in.
>
> I have checked in lock.c file for the message where lock tags have been used.
> elog(ERROR, "lock %s on object %u/%u/%u is already held",
>      lockMethodTable->lockModeNames[lockmode],
>      lock->tag.locktag_field1, lock->tag.locktag_field2,
>      lock->tag.locktag_field3);
>
> This can give more information about erroneous lock.

A better error message would be nice, but I don't think it's worth that 
much. resowner.c doesn't currently know about the internals of LOCALLOCk 
struct or lock tags, and I'd prefer to keep it that way. Let's just 
print the pointer's address, that's what we do in many other 
corresponding error messages in other Forget* functions.

On 11.06.2012 18:21, Jeff Janes wrote:
> On Sun, Jun 10, 2012 at 11:28 PM, Amit Kapila<amit.kapila <at> huawei.com>  wrote:
>> 2. ResourceOwnerForgetLock(), it decrements the lock count before removing,
>> so incase it doesn't find the lock in lockarray, the count will be
>> decremented inspite the array still contains the same number of locks.
 >
> Should it emit a FATAL rather than an ERROR?  I thought ERROR was
(Continue reading)

Permalink | Reply |
headers
Amit Kapila | 18 Jun 2012 16:31
Favicon

Re: Resource Owner reassign Locks

> A better error message would be nice, but I don't think it's worth that 
> much. resowner.c doesn't currently know about the internals of LOCALLOCk 
> struct or lock tags, and I'd prefer to keep it that way. Let's just 
> print the pointer's address, that's what we do in many other 
> corresponding error messages in other Forget* functions.

I have checked there also doesn't exist any functions which expose lock
internal parameters like tag values.
So we can modify as you suggested.

-----Original Message-----
From: Heikki Linnakangas [mailto:heikki.linnakangas <at> enterprisedb.com] 
Sent: Monday, June 18, 2012 4:25 PM
To: Amit kapila; Jeff Janes
Cc: pgsql-hackers
Subject: Re: Resource Owner reassign Locks

On 16.06.2012 09:04, Amit kapila wrote:
>> I don't think so.  C doesn't ref count its pointers.
> You are right I have misunderstood.
>
>> I don't think that lock tags have good human readable formats, and just
>> a pointer dump probably wouldn't be much use when something that can
>> never happen has happened.  But I'll at least add a reference to the
>> resource owner if this stays in.
>
> I have checked in lock.c file for the message where lock tags have been
used.
> elog(ERROR, "lock %s on object %u/%u/%u is already held",
>      lockMethodTable->lockModeNames[lockmode],
(Continue reading)

Permalink | Reply |

Gmane