Re: [PATCH] lock_timeout and common SIGALRM framework

From: Boszormenyi Zoltan <zb(at)cybertec(dot)at>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Cousin Marc <cousinmarc(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)commandprompt(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Hans-Juergen Schoenig <hs(at)cybertec(dot)at>, Ants Aasma <ants(at)cybertec(dot)at>
Subject: Re: [PATCH] lock_timeout and common SIGALRM framework
Date: 2012-04-08 16:43:45
Message-ID: 4F81C041.40800@cybertec.at
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

2012-04-08 11:24 keltezéssel, Boszormenyi Zoltan írta:
> 2012-04-06 14:47 keltezéssel, Cousin Marc írta:
>> On 05/04/12 08:02, Boszormenyi Zoltan wrote:
>>> 2012-04-04 21:30 keltezéssel, Alvaro Herrera írta:
>>>> I think this patch is doing two things: first touching infrastructure
>>>> stuff and then adding lock_timeout on top of that. Would it work to
>>>> split the patch in two pieces?
>>>>
>>> Sure. Attached is the split version.
>>>
>>> Best regards,
>>> Zoltán Böszörményi
>>>
>> Hi,
>>
>> I've started looking at and testing both patches.
>>
>> Technically speaking, I think the source looks much better than the
>> first version of lock timeout, and may help adding other timeouts in the
>> future.
>
> Thanks.
>
>> I haven't tested it in depth though, because I encountered the
>> following problem:
>>
>> While testing the patch, I found a way to crash PG. But what's weird is
>> that it crashes also with an unpatched git version.
>>
>> Here is the way to reproduce it (I have done it with a pgbench schema):
>>
>> - Set a small statement_timeout (just to save time during the tests)
>>
>> Session1:
>> =#BEGIN;
>> =#lock TABLE pgbench_accounts ;
>>
>> Session 2:
>> =#BEGIN;
>> =#lock TABLE pgbench_accounts ;
>> ERROR: canceling statement due to statement timeout
>> =# lock TABLE pgbench_accounts ;
>>
>> I'm using \set ON_ERROR_ROLLBACK INTERACTIVE by the way. It can also be
>> done with a rollback to savepoint of course.
>>
>> Session 2 crashes with this : TRAP : FailedAssertion(«
>> !(locallock->holdsStrongLockCount == 0) », fichier : « lock.c », ligne :
>> 749).
>>
>> It can also be done without a statement_timeout, and a control-C on the
>> second lock table.
>
> Indeed, the unpatched GIT version crashes if you enter
> =#lock TABLE pgbench_accounts ;
> the second time in session 2 after the first one failed. Also,
> manually spelling it out:
>
> Session 1:
>
> $ psql
> psql (9.2devel)
> Type "help" for help.
>
> zozo=# begin;
> BEGIN
> zozo=# lock table pgbench_accounts;
> LOCK TABLE
> zozo=#
>
> Session 2:
>
> zozo=# begin;
> BEGIN
> zozo=# savepoint a;
> SAVEPOINT
> zozo=# lock table pgbench_accounts;
> ERROR: canceling statement due to statement timeout
> zozo=# rollback to a;
> ROLLBACK
> zozo=# savepoint b;
> SAVEPOINT
> zozo=# lock table pgbench_accounts;
> The connection to the server was lost. Attempting reset: Failed.
> !>
>
> Server log after the second lock table:
>
> TRAP: FailedAssertion("!(locallock->holdsStrongLockCount == 0)", File: "lock.c", Line: 749)
> LOG: server process (PID 12978) was terminated by signal 6: Aborted
>
> Best regards,
> Zoltán Böszörményi

Robert, the Assert triggering with the above procedure
is in your "fast path" locking code with current GIT.

Best regards,
Zoltán Böszörményi

>
>>
>> I didn't touch anything but this. It occurs everytime, when asserts are
>> activated.
>>
>> I tried it on 9.1.3, and I couldn't make it crash with the same sequence
>> of events. So maybe it's something introduced since ? Or is the assert
>> still valid ?
>>
>> Cheers
>>
>
>

--
----------------------------------
Zoltán Böszörményi
Cybertec Schönig& Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt, Austria
Web: http://www.postgresql-support.de
http://www.postgresql.at/

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2012-04-08 16:53:04 Re: patch: improve SLRU replacement algorithm
Previous Message Boszormenyi Zoltan 2012-04-08 16:35:33 Re: ECPG FETCH readahead