Thread: BUG #6629: Creating a gist index fails with "too many LWLocks taken"

The following bug has been logged on the website:

Bug reference:      6629
Logged by:          Tom Forbes
Email address:      tom@tomforb.es
PostgreSQL version: 9.1.3
Operating system:   Windows 7 64bit
Description:=20=20=20=20=20=20=20=20

On a test database with 10,000 rows, each containing a array of 50 unique
random integers from 0 to 1000 creating a gist index on the column with
gist__int_ops as an option fails with the following error:
"too many LWLocks taken".

My application is not leaking locks: running this through pgadmin results in
the same error. Creating the index without gist__int_ops or with
gist__intbig_ops succeeds however. Creating the index on the same number of
rows with only 25 elements succeeds.

The documentation (http://www.postgresql.org/docs/9.1/static/intarray.html)
states that using gist__intbig_ops is "more suitable for indexing large
numbers of distinct array values" not that gist__int_ops cannot handle large
numbers of distinct values.

I have extracted out some code from the project I was working on that
reproduces the problem on my machine. You can find it here:
http://nopaste.snit.ch/138083

On 05.05.2012 22:49, tom@tomforb.es wrote:
> The following bug has been logged on the website:
>
> Bug reference:      6629
> Logged by:          Tom Forbes
> Email address:      tom@tomforb.es
> PostgreSQL version: 9.1.3
> Operating system:   Windows 7 64bit
> Description:
>
> On a test database with 10,000 rows, each containing a array of 50 unique
> random integers from 0 to 1000 creating a gist index on the column with
> gist__int_ops as an option fails with the following error:
> "too many LWLocks taken".

I modified the way GiST page splitting works in 9.1, this seems to be
caused by those changes. When a page is split and the downlink for the
new page is inserted to the parent, we keep a lock on the child and the
parent. But inserting the downlink to the parent can cause the parent to
split too, and so forth, all the way to the root. There's a hard-coded
limit that a backend can hold at most 100 lwlocks simultaneously, and
what happens is that when the tree is very tall, about 50 levels tall in
this case, you run into that limit when you have to do a page split at
every level.

We could rearrange the page splitting algorithm to release locks
earlier, before traversing to the next parent level. I didn't do that
because I thought no-one would create an index that tall and the code
was a bit easier to follow when locks are released in the same function
where they're acquired, but looks like I was wrong. I'm not sure how
useful such an index is in practice, but at least it's apparently easy
to create one.

I wrote a quick patch to do that, and with the patch the index build
finished - but it took hours. And the index was 10GB in size, where the
heap is just 12 MB, and searches using the index take ages. Do you have
a real-life scenario where you run into this limit? I'm a bit reluctant
to change the code unless there's an actual use case for a gist index
more than 50 levels deep.

--
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com

Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes:
> We could rearrange the page splitting algorithm to release locks
> earlier, before traversing to the next parent level.

That seems like a good idea just on concurrency grounds; I'm worried
about both the performance implications and the risk of deadlock.

> I wrote a quick patch to do that, and with the patch the index build
> finished - but it took hours. And the index was 10GB in size, where the
> heap is just 12 MB, and searches using the index take ages.

Hm, is the example exploiting some pessimal behavior in the picksplit
logic for the particular opclass?  Maybe that's something to fix, too.

            regards, tom lane

Nope, this was just a benchmark script that caused this, any sane person
would use an intbig index instead I guess. A better error message would be
nice though, I was pretty confused when this happened.

This can also bring down postgresql - it happens occasionally and causes
the server to terminate. Someone in #postgresql said this happens when the
failure to acquire the lock occurs in a "critical section"? That might be
cause for concern.

~Tom

On Mon, May 7, 2012 at 4:37 PM, Heikki Linnakangas <
heikki.linnakangas@enterprisedb.com> wrote:

> On 05.05.2012 22:49, tom@tomforb.es wrote:
>
>> The following bug has been logged on the website:
>>
>> Bug reference:      6629
>> Logged by:          Tom Forbes
>> Email address:      tom@tomforb.es
>> PostgreSQL version: 9.1.3
>> Operating system:   Windows 7 64bit
>> Description:
>>
>> On a test database with 10,000 rows, each containing a array of 50 unique
>> random integers from 0 to 1000 creating a gist index on the column with
>> gist__int_ops as an option fails with the following error:
>> "too many LWLocks taken".
>>
>
> I modified the way GiST page splitting works in 9.1, this seems to be
> caused by those changes. When a page is split and the downlink for the new
> page is inserted to the parent, we keep a lock on the child and the parent.
> But inserting the downlink to the parent can cause the parent to split too,
> and so forth, all the way to the root. There's a hard-coded limit that a
> backend can hold at most 100 lwlocks simultaneously, and what happens is
> that when the tree is very tall, about 50 levels tall in this case, you run
> into that limit when you have to do a page split at every level.
>
> We could rearrange the page splitting algorithm to release locks earlier,
> before traversing to the next parent level. I didn't do that because I
> thought no-one would create an index that tall and the code was a bit
> easier to follow when locks are released in the same function where they're
> acquired, but looks like I was wrong. I'm not sure how useful such an index
> is in practice, but at least it's apparently easy to create one.
>
> I wrote a quick patch to do that, and with the patch the index build
> finished - but it took hours. And the index was 10GB in size, where the
> heap is just 12 MB, and searches using the index take ages. Do you have a
> real-life scenario where you run into this limit? I'm a bit reluctant to
> change the code unless there's an actual use case for a gist index more
> than 50 levels deep.
>
> --
>  Heikki Linnakangas
>  EnterpriseDB   http://www.enterprisedb.com
>

On Mon, May 07, 2012 at 05:31:40PM +0100, tom Tom wrote:
> Nope, this was just a benchmark script that caused this, any sane person
> would use an intbig index instead I guess. A better error message would be
> nice though, I was pretty confused when this happened.
>
> This can also bring down postgresql - it happens occasionally and causes
> the server to terminate. Someone in #postgresql said this happens when the
> failure to acquire the lock occurs in a "critical section"? That might be
> cause for concern.
Occasionally, it causes a PANIC instead of an ERROR. I have the logs
from the IRC session if anyone is in need of them.

>
> ~Tom
>
> On Mon, May 7, 2012 at 4:37 PM, Heikki Linnakangas <
> heikki.linnakangas@enterprisedb.com> wrote:
>
> > On 05.05.2012 22:49, tom@tomforb.es wrote:
> >
> >> The following bug has been logged on the website:
> >>
> >> Bug reference:      6629
> >> Logged by:          Tom Forbes
> >> Email address:      tom@tomforb.es
> >> PostgreSQL version: 9.1.3
> >> Operating system:   Windows 7 64bit
> >> Description:
> >>
> >> On a test database with 10,000 rows, each containing a array of 50 unique
> >> random integers from 0 to 1000 creating a gist index on the column with
> >> gist__int_ops as an option fails with the following error:
> >> "too many LWLocks taken".
> >>
> >
> > I modified the way GiST page splitting works in 9.1, this seems to be
> > caused by those changes. When a page is split and the downlink for the new
> > page is inserted to the parent, we keep a lock on the child and the parent.
> > But inserting the downlink to the parent can cause the parent to split too,
> > and so forth, all the way to the root. There's a hard-coded limit that a
> > backend can hold at most 100 lwlocks simultaneously, and what happens is
> > that when the tree is very tall, about 50 levels tall in this case, you run
> > into that limit when you have to do a page split at every level.
> >
> > We could rearrange the page splitting algorithm to release locks earlier,
> > before traversing to the next parent level. I didn't do that because I
> > thought no-one would create an index that tall and the code was a bit
> > easier to follow when locks are released in the same function where they're
> > acquired, but looks like I was wrong. I'm not sure how useful such an index
> > is in practice, but at least it's apparently easy to create one.
> >
> > I wrote a quick patch to do that, and with the patch the index build
> > finished - but it took hours. And the index was 10GB in size, where the
> > heap is just 12 MB, and searches using the index take ages. Do you have a
> > real-life scenario where you run into this limit? I'm a bit reluctant to
> > change the code unless there's an actual use case for a gist index more
> > than 50 levels deep.
> >
> > --
> >  Heikki Linnakangas
> >  EnterpriseDB   http://www.enterprisedb.com
> >

-Ryan Kelly

On 07.05.2012 18:51, Tom Lane wrote:
> Heikki Linnakangas<heikki.linnakangas@enterprisedb.com>  writes:
>> I wrote a quick patch to do that, and with the patch the index build
>> finished - but it took hours. And the index was 10GB in size, where the
>> heap is just 12 MB, and searches using the index take ages.
>
> Hm, is the example exploiting some pessimal behavior in the picksplit
> logic for the particular opclass?  Maybe that's something to fix, too.

Yep. I added an elog() to picksplit to print how many tuples went to
left and right pages. This seems to be the same behavior described here:
http://archives.postgresql.org/pgsql-performance/2009-04/msg00320.php,
where one entry goes to one page, and all the rest to the other page. We
changed the picksplit algorithm for 'seg' to fix that. I'm not too
familiar with these datatypes, I'm not sure if we could do the same for
intarray.

Tom F's original test case used arrays of 50 integers, but you can see
the same with much smaller arrays, too. Here's a snippet of the output
with arrays of five integers:

LOG:  --------picksplit 1 - 9
LOG:  --------picksplit 1 - 9
LOG:  --------picksplit 9 - 1
LOG:  --------picksplit 1 - 9
LOG:  --------picksplit 60 - 63
LOG:  --------picksplit 1 - 9
LOG:  --------picksplit 9 - 1
LOG:  --------picksplit 1 - 9
LOG:  --------picksplit 1 - 9
LOG:  --------picksplit 9 - 1
LOG:  --------picksplit 1 - 9
LOG:  --------picksplit 1 - 10
LOG:  --------picksplit 1 - 9
LOG:  --------picksplit 1 - 9
LOG:  --------picksplit 1 - 10
LOG:  --------picksplit 9 - 1
LOG:  --------picksplit 1 - 9
LOG:  --------picksplit 1 - 9
LOG:  --------picksplit 1 - 10
LOG:  --------picksplit 10 - 1
LOG:  --------picksplit 1 - 10
LOG:  --------picksplit 9 - 1
LOG:  --------picksplit 1 - 9
LOG:  --------picksplit 1 - 9
LOG:  --------picksplit 10 - 1
LOG:  --------picksplit 1 - 9
LOG:  --------picksplit 1 - 9
LOG:  --------picksplit 1 - 9
LOG:  --------picksplit 9 - 1
LOG:  --------picksplit 1 - 9
LOG:  --------picksplit 1 - 9
LOG:  --------picksplit 9 - 1
LOG:  --------picksplit 1 - 9
LOG:  --------picksplit 1 - 9
LOG:  --------picksplit 9 - 1
LOG:  --------picksplit 1 - 9
LOG:  --------picksplit 61 - 62
LOG:  --------picksplit 9 - 1
...

--
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com

On 07.05.2012 18:51, Tom Lane wrote:
> Heikki Linnakangas<heikki.linnakangas@enterprisedb.com>  writes:
>> We could rearrange the page splitting algorithm to release locks
>> earlier, before traversing to the next parent level.
>
> That seems like a good idea just on concurrency grounds; I'm worried
> about both the performance implications and the risk of deadlock.

Ok, committed a patch to release locks earlier when recursing up the tree.

This still doesn't completely eliminate the problem: when a page is
split into more than two halves, the downlinks are inserted separately
for each of the extra right pages. While that's done, the rest of the
siblings are kept locked. So in effect, we still have the same issue
when all the splits are 3 (or more)-way splits. I'm not going to try
fixing that now, because it's an exceedingly rare corner-case, and would
be rather difficult to fix.

--
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com

On 08.05.2012 04:15, Ryan Kelly wrote:
> On Mon, May 07, 2012 at 05:31:40PM +0100, tom Tom wrote:
>> Nope, this was just a benchmark script that caused this, any sane person
>> would use an intbig index instead I guess. A better error message would be
>> nice though, I was pretty confused when this happened.
>>
>> This can also bring down postgresql - it happens occasionally and causes
>> the server to terminate. Someone in #postgresql said this happens when the
>> failure to acquire the lock occurs in a "critical section"? That might be
>> cause for concern.
> Occasionally, it causes a PANIC instead of an ERROR. I have the logs
> from the IRC session if anyone is in need of them.

Good point.

I wonder if we should reserve a few of the lwlock "slots" for critical
sections, to make this less likely to happen. Not only in this case, but
in general. We haven't seen this problem often, but it would be quite
trivial to reserve a few slots.

--
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com

Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes:
> I wonder if we should reserve a few of the lwlock "slots" for critical
> sections, to make this less likely to happen. Not only in this case, but
> in general. We haven't seen this problem often, but it would be quite
> trivial to reserve a few slots.

I'm against that: it would complicate a performance-critical and
correctness-critical part of the code, in return for what exactly?
IMO, no part of the system should ever get within an order of magnitude
of holding 100 LWLocks concurrently.  For one thing, I don't believe
it's possible to statically guarantee no deadlock once things get that
messy; and for another, it'd surely be horrible from a concurrency
standpoint.  So anytime we find something that's reaching that limit,
the solution is to fix the lock usage, not to make the limit more
forgiving.

            regards, tom lane

On 11 May 2012 11:07, Heikki Linnakangas
<heikki.linnakangas@enterprisedb.com> wrote:

> I wonder if we should reserve a few of the lwlock "slots" for critical
> sections, to make this less likely to happen. Not only in this case, but =
in
> general. We haven't seen this problem often, but it would be quite trivial
> to reserve a few slots.

Why reserve them solely for critical sections?

What is the downside from having >100 slots for general use.

ISTM we should have 250 slots and log a warning if we ever hit 50 or more.

--=20
=A0Simon Riggs=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 http:/=
/www.2ndQuadrant.com/
=A0PostgreSQL Development, 24x7 Support, Training & Services

On 11.05.2012 16:52, Tom Lane wrote:
> Heikki Linnakangas<heikki.linnakangas@enterprisedb.com>  writes:
>> I wonder if we should reserve a few of the lwlock "slots" for critical
>> sections, to make this less likely to happen. Not only in this case, but
>> in general. We haven't seen this problem often, but it would be quite
>> trivial to reserve a few slots.
>
> I'm against that: it would complicate a performance-critical and
> correctness-critical part of the code, in return for what exactly?
> IMO, no part of the system should ever get within an order of magnitude
> of holding 100 LWLocks concurrently.

I agree we should never get anywhere near that limit. But if we do -
because of another bug like this one - it would be nice if it was just
an ERROR, instead of a PANIC.

> For one thing, I don't believe
> it's possible to statically guarantee no deadlock once things get that
> messy; and for another, it'd surely be horrible from a concurrency
> standpoint.

Well, for example in the case of a gist page split that splits a page
into a hundred pages, all but one of the pages involved is previously
unused. It's quite easy to guarantee that's deadlock free. It's
nevertheless not a good idea in practice to do that, of course.

--
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com

On 11.05.2012 16:56, Simon Riggs wrote:
> On 11 May 2012 11:07, Heikki Linnakangas
> <heikki.linnakangas@enterprisedb.com>  wrote:
>
>> I wonder if we should reserve a few of the lwlock "slots" for critical
>> sections, to make this less likely to happen. Not only in this case, but in
>> general. We haven't seen this problem often, but it would be quite trivial
>> to reserve a few slots.
>
> Why reserve them solely for critical sections?

Because if you run out of lwlocks in a critical section, you get a PANIC.

> What is the downside from having>100 slots for general use.
>
> ISTM we should have 250 slots and log a warning if we ever hit 50 or more.

Then we would be back to square one, if a piece of code acquires 250
locks, then enters a critical section, and tries to acquire one more
lock -> PANIC.

--
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com

Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes:
> On 11.05.2012 16:52, Tom Lane wrote:
>> IMO, no part of the system should ever get within an order of magnitude
>> of holding 100 LWLocks concurrently.

> I agree we should never get anywhere near that limit. But if we do -
> because of another bug like this one - it would be nice if it was just
> an ERROR, instead of a PANIC.

By the time you hit that limit, you have already got a problem that
should never have gotten into the field, I think.  Simon's idea of
logging a warning once we get beyond a sane number of LWLocks seems like
it might be helpful towards finding such problems earlier; though I'd
put the "sane" limit at maybe 20 or so.  Perhaps it'd be useful to
measure what the max length of that list is during the regression tests.

            regards, tom lane

On Fri, May 11, 2012 at 10:19 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes:
>> On 11.05.2012 16:52, Tom Lane wrote:
>>> IMO, no part of the system should ever get within an order of magnitude
>>> of holding 100 LWLocks concurrently.
>
>> I agree we should never get anywhere near that limit. But if we do -
>> because of another bug like this one - it would be nice if it was just
>> an ERROR, instead of a PANIC.
>
> By the time you hit that limit, you have already got a problem that
> should never have gotten into the field, I think. =A0Simon's idea of
> logging a warning once we get beyond a sane number of LWLocks seems like
> it might be helpful towards finding such problems earlier; though I'd
> put the "sane" limit at maybe 20 or so.

+1.

> Perhaps it'd be useful to
> measure what the max length of that list is during the regression tests.

Yeah.

And maybe any build with --enable-cassert should also emit WARNINGs
when we go past whatever we determine the same limit to be.

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

On 11 May 2012 15:14, Heikki Linnakangas
<heikki.linnakangas@enterprisedb.com> wrote:
> On 11.05.2012 16:56, Simon Riggs wrote:
>>
>> On 11 May 2012 11:07, Heikki Linnakangas
>> <heikki.linnakangas@enterprisedb.com> =A0wrote:
>>
>>> I wonder if we should reserve a few of the lwlock "slots" for critical
>>> sections, to make this less likely to happen. Not only in this case, but
>>> in
>>> general. We haven't seen this problem often, but it would be quite
>>> trivial
>>> to reserve a few slots.
>>
>>
>> Why reserve them solely for critical sections?
>
>
> Because if you run out of lwlocks in a critical section, you get a PANIC.

Yes, but why reserve them solely for critical sections? If you have an
escape hatch you use it, always


--=20
=A0Simon Riggs=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 http:/=
/www.2ndQuadrant.com/
=A0PostgreSQL Development, 24x7 Support, Training & Services

On 11.05.2012 18:18, Simon Riggs wrote:
> On 11 May 2012 15:14, Heikki Linnakangas
> <heikki.linnakangas@enterprisedb.com>  wrote:
>> On 11.05.2012 16:56, Simon Riggs wrote:
>>>
>>> On 11 May 2012 11:07, Heikki Linnakangas
>>> <heikki.linnakangas@enterprisedb.com>    wrote:
>>>
>>>> I wonder if we should reserve a few of the lwlock "slots" for critical
>>>> sections, to make this less likely to happen. Not only in this case, but
>>>> in
>>>> general. We haven't seen this problem often, but it would be quite
>>>> trivial
>>>> to reserve a few slots.
>>>
>>>
>>> Why reserve them solely for critical sections?
>>
>> Because if you run out of lwlocks in a critical section, you get a PANIC.
>
> Yes, but why reserve them solely for critical sections? If you have an
> escape hatch you use it, always

Well, no, because a PANIC is much worse than an ERROR. Think of this as
a spare oxygen tank while diving, rather than an escape hatch. A spare
tank can save your life if you run out of oxygen while ascending, but if
you run out of oxygen on the way down, you don't continue going down
with just the spare tank.

Imagine that you have a process that does something like this:

for (i=0; i < 99; i++)
   LWLockAcquire(foolock[i])

START_CRIT_SECTION();

XLogInsert(...)

END_CRIT_SECTION();

What happens at the moment is that XLogInsert hits the limit when it
tries to acquire WALInsertLock, so you get a PANIC. If we reserved, say,
5 locks for critical sections, so that you could hold 95 locks while
outside a critical section, and 5 more within on, you would get an error
earlier, outside the critical section, while acquiring the "foolocks".
Or if the number of foolocks acquired was less than 95, you would not
get error at all. That avoids the PANIC.

You can argue for just raising the limit, but that just moves the
problem around. It's still possible to hit the limit within a critical
section, and PANIC. Likewise, if we lower the limit, that helps us find
the problems earlier in the development cycle, but doesn't change the
fact that if you run too close to the edge, you run out of locks within
a critical section and PANIC.

Of course, nothing stops you from writing (bad) code that acquires a lot
of locks within a critical section, in which case you're screwed anyway.

--
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com

On 11 May 2012 17:48, Heikki Linnakangas
<heikki.linnakangas@enterprisedb.com> wrote:
> On 11.05.2012 18:18, Simon Riggs wrote:
>>
>> On 11 May 2012 15:14, Heikki Linnakangas
>> <heikki.linnakangas@enterprisedb.com> =A0wrote:
>>>
>>> On 11.05.2012 16:56, Simon Riggs wrote:
>>>>
>>>>
>>>> On 11 May 2012 11:07, Heikki Linnakangas
>>>> <heikki.linnakangas@enterprisedb.com> =A0 =A0wrote:
>>>>
>>>>> I wonder if we should reserve a few of the lwlock "slots" for critical
>>>>> sections, to make this less likely to happen. Not only in this case,
>>>>> but
>>>>> in
>>>>> general. We haven't seen this problem often, but it would be quite
>>>>> trivial
>>>>> to reserve a few slots.
>>>>
>>>>
>>>>
>>>> Why reserve them solely for critical sections?
>>>
>>>
>>> Because if you run out of lwlocks in a critical section, you get a PANI=
C.
>>
>>
>> Yes, but why reserve them solely for critical sections? If you have an
>> escape hatch you use it, always
>
>
> Well, no, because a PANIC is much worse than an ERROR. Think of this as a
> spare oxygen tank while diving, rather than an escape hatch. A spare tank
> can save your life if you run out of oxygen while ascending, but if you r=
un
> out of oxygen on the way down, you don't continue going down with just the
> spare tank.
>
> Imagine that you have a process that does something like this:
>
> for (i=3D0; i < 99; i++)
> =A0LWLockAcquire(foolock[i])
>
> START_CRIT_SECTION();
>
> XLogInsert(...)
>
> END_CRIT_SECTION();
>
> What happens at the moment is that XLogInsert hits the limit when it tries
> to acquire WALInsertLock, so you get a PANIC. If we reserved, say, 5 locks
> for critical sections, so that you could hold 95 locks while outside a
> critical section, and 5 more within on, you would get an error earlier,
> outside the critical section, while acquiring the "foolocks". Or if the
> number of foolocks acquired was less than 95, you would not get error at
> all. That avoids the PANIC.
>
> You can argue for just raising the limit, but that just moves the problem
> around. It's still possible to hit the limit within a critical section, a=
nd
> PANIC. Likewise, if we lower the limit, that helps us find the problems
> earlier in the development cycle, but doesn't change the fact that if you
> run too close to the edge, you run out of locks within a critical section
> and PANIC.
>
> Of course, nothing stops you from writing (bad) code that acquires a lot =
of
> locks within a critical section, in which case you're screwed anyway.

OK, I agree.

User code can create and hold lwlocks as it chooses, so we need to
protect the server as a whole from bad user code.

The implementation I would prefer would be to put the check in
START_CRIT_SECTION(); so we actually fail before we enter the section.
That way we don't need extra locks, which is good since any additional
amount we pick can still be exceeded by user code.

--=20
=A0Simon Riggs=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 http:/=
/www.2ndQuadrant.com/
=A0PostgreSQL Development, 24x7 Support, Training & Services

Simon Riggs <simon@2ndQuadrant.com> writes:
> OK, I agree.

I still think this is about solving a non-problem.

> The implementation I would prefer would be to put the check in
> START_CRIT_SECTION(); so we actually fail before we enter the section.

... and this is proposing adding significant overhead to solve a
non-problem.

The suggested warning can be added without any new cycles in the normal
case, ie, where we currently have

    if (lockcount >= 100)
        elog(ERROR, ...);

we can do something like

    if (lockcount >= 20)
    {
        if (lockcount >= 100)
            elog(ERROR, ...);
        else
            elog(WARNING, ...);
    }

I think that's more than sufficient.  We've had the LWLock mechanism
for umpteen years now, and this hasn't come up before, and now that
it has come up it was because of a locking usage that needed to be
rewritten anyway.  That is *not* good evidence for adding complexity
and cycles to the mechanism to make it more failsoft.  It shouldn't
need to be failsoft.

            regards, tom lane