Horton

Thread

Commits

  1. Fix check for conflicting session- vs transaction-level locks.

  1. BUG #17122: panic on prepare with subsequent pg_advisory_lock() and pg_advisory_xact_lock_shared()

    The Post Office <noreply@postgresql.org> — 2021-07-23T14:17:25Z

    The following bug has been logged on the website:

Bug reference:      17122
Logged by:          Alexander Pyhalov
Email address:      a.pyhalov@postgrespro.ru
PostgreSQL version: 13.3
Operating system:   Ubuntu 20.04
Description:        

The issue is reproducible at least on  on PG 13.3 and 14beta2

mkdir d;
initdb -D d;

Increase max_prepared_transactions in postgresql.conf  (for example, set it
to 10).

The following statements lead to panic:
begin;
select pg_advisory_lock(1);
select pg_advisory_xact_lock_shared(1);
prepare transaction 'test';

2021-07-23 17:15:59.868 MSK [61851] PANIC:  we seem to have dropped a bit
somewhere
2021-07-23 17:15:59.868 MSK [61851] STATEMENT:  prepare transaction
'test';

(gdb) bt
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1  0x00007fc72923c859 in __GI_abort () at abort.c:79
#2  0x000055f80d4facdd in errfinish (filename=0x55f80d6b53c3 "lock.c",
lineno=3466, funcname=0x55f80d6b6200 <__func__.13837> "PostPrepare_Locks")
at elog.c:592
#3  0x000055f80d340143 in PostPrepare_Locks (xid=485) at lock.c:3466
#4  0x000055f80cf50942 in PrepareTransaction () at xact.c:2519
#5  0x000055f80cf512b9 in CommitTransactionCommand () at xact.c:3034
#6  0x000055f80d35c487 in finish_xact_command () at postgres.c:2662
#7  0x000055f80d359edb in exec_simple_query (query_string=0x55f80f0890a0
"prepare transaction 'test';") at postgres.c:1264
#8  0x000055f80d35e587 in PostgresMain (argc=1, argv=0x55f80f0b38b0,
dbname=0x55f80f0856d8 "postgres", username=0x55f80f0b37d0 "leoric") at
postgres.c:4339
#9  0x000055f80d29ddf4 in BackendRun (port=0x55f80f0abc20) at
postmaster.c:4526
#10 0x000055f80d29d4f6 in BackendStartup (port=0x55f80f0abc20) at
postmaster.c:4210
#11 0x000055f80d2997ec in ServerLoop () at postmaster.c:1739
#12 0x000055f80d298f85 in PostmasterMain (argc=3, argv=0x55f80f083650) at
postmaster.c:1412
#13 0x000055f80d193dcd in main (argc=3, argv=0x55f80f083650) at main.c:210

  2. Re: BUG #17122: panic on prepare with subsequent pg_advisory_lock() and pg_advisory_xact_lock_shared()

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-07-23T16:12:46Z

    PG Bug reporting form <noreply@postgresql.org> writes:
> The following statements lead to panic:
> begin;
> select pg_advisory_lock(1);
> select pg_advisory_xact_lock_shared(1);
> prepare transaction 'test';

Thanks for the report!  Looks like the shared lock is sufficient
to cause the problem:

regression=# begin;
BEGIN
regression=*# select pg_advisory_xact_lock_shared(1);
 pg_advisory_xact_lock_shared 
------------------------------
 
(1 row)

regression=*# prepare transaction 'test';
PANIC:  we seem to have dropped a bit somewhere
server closed the connection unexpectedly

Taking only the other lock works fine.  Even more interesting,
"select pg_advisory_xact_lock(1);" also works, as does
"select pg_advisory_lock_shared(1);".  So it's specific to
xact-level shared locks, which seems downright weird.

I also tried this:

regression=# begin;
BEGIN
regression=*# select pg_advisory_lock_shared(1);
 pg_advisory_lock_shared 
-------------------------
 
(1 row)

regression=*# select pg_advisory_xact_lock(1);
 pg_advisory_xact_lock 
-----------------------
 
(1 row)

regression=*# select pg_advisory_lock(1);
 pg_advisory_lock 
------------------
 
(1 row)

regression=*# prepare transaction 'test';
ERROR:  cannot PREPARE while holding both session-level and transaction-level locks on the same object

which makes me wonder why we didn't issue that error in your
example case.  However, that's not sufficient to explain
the crash with only the xact-shared lock.

BTW, note that we do manage to prepare the transaction before
crashing, so you must do
commit prepared 'test';
to clean up before trying again.

			regards, tom lane

  3. Re: BUG #17122: panic on prepare with subsequent pg_advisory_lock() and pg_advisory_xact_lock_shared()

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-07-24T21:44:57Z

    I wrote:
> PG Bug reporting form <noreply@postgresql.org> writes:
>> The following statements lead to panic:
>> begin;
>> select pg_advisory_lock(1);
>> select pg_advisory_xact_lock_shared(1);
>> prepare transaction 'test';

> Thanks for the report!  Looks like the shared lock is sufficient
> to cause the problem:

Ah, scratch that.  I can't reproduce it now, and I think I messed
up yesterday by not remembering that pg_advisory_lock() takes a
session-level lock that would continue to be held after PREPARE.

> I also tried this:
> ...
> regression=*# prepare transaction 'test';
> ERROR:  cannot PREPARE while holding both session-level and transaction-level locks on the same object
> which makes me wonder why we didn't issue that error in your
> example case.

The reason why not is that the code that's meant to detect that is
just fundamentally inadequate.  It's examining individual LOCALLOCK
entries to detect conflicts, but we keep separate LOCALLOCK entries
for different lockmodes on the same object.  So pg_advisory_lock()
creates an entry with lockmode ExclusiveLock, while
pg_advisory_xact_lock_shared() creates a different entry with lockmode
AccessShareLock.  But they are pointing at the same PROCLOCK, and the
restriction we're dealing with here applies at the PROCLOCK level.

So I think we need something like the attached to fix this.

In the longer term, maybe it'd be better to rethink how we represent
LOCALLOCK entries so that they are one-to-one with PROCLOCKs.  But
rearranging that data structure for the convenience of PREPARE
TRANSACTION is probably going to be a net loss performance-wise.
In any case, we certainly wouldn't risk back-patching such a change.

			regards, tom lane

  4. Re: BUG #17122: panic on prepare with subsequent pg_advisory_lock() and pg_advisory_xact_lock_shared()

    x4mmm@yandex-team.ru — 2021-07-25T09:34:46Z

    
> 25 июля 2021 г., в 02:44, Tom Lane <tgl@sss.pgh.pa.us> написал(а):
> 
> +PREPARE TRANSACTION 'foo6';  -- fails
> +ERROR:  prepared transactions are disabled
> +HINT:  Set max_prepared_transactions to a nonzero value.

Do we actually expect this particular error?

Best regards, Andrey Borodin.

  5. Re: BUG #17122: panic on prepare with subsequent pg_advisory_lock() and pg_advisory_xact_lock_shared()

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-07-25T14:32:06Z

    Andrey Borodin <x4mmm@yandex-team.ru> writes:
>> 25 июля 2021 г., в 02:44, Tom Lane <tgl@sss.pgh.pa.us> написал(а):
>> +PREPARE TRANSACTION 'foo6';  -- fails
>> +ERROR:  prepared transactions are disabled
>> +HINT:  Set max_prepared_transactions to a nonzero value.

> Do we actually expect this particular error?

Of course.  "make installcheck" will produce that result file,
unless you've changed max_prepared_transactions on the server
being tested.

			regards, tom lane

  6. Re: BUG #17122: panic on prepare with subsequent pg_advisory_lock() and pg_advisory_xact_lock_shared()

    x4mmm@yandex-team.ru — 2021-07-25T15:54:29Z

    
> 25 июля 2021 г., в 19:32, Tom Lane <tgl@sss.pgh.pa.us> написал(а):
> 
> Andrey Borodin <x4mmm@yandex-team.ru> writes:
>>> 25 июля 2021 г., в 02:44, Tom Lane <tgl@sss.pgh.pa.us> написал(а):
>>> +PREPARE TRANSACTION 'foo6';  -- fails
>>> +ERROR:  prepared transactions are disabled
>>> +HINT:  Set max_prepared_transactions to a nonzero value.
> 
>> Do we actually expect this particular error?
> 
> Of course.  "make installcheck" will produce that result file,
> unless you've changed max_prepared_transactions on the server
> being tested.

Sorry for the noise. I did not know that suffixes for .out files work this way.

Best regards, Andrey Borodin.

  7. Re: BUG #17122: panic on prepare with subsequent pg_advisory_lock() and pg_advisory_xact_lock_shared()

    Michael Paquier <michael@paquier.xyz> — 2021-07-26T03:31:32Z

    On Sat, Jul 24, 2021 at 05:44:57PM -0400, Tom Lane wrote:
> The reason why not is that the code that's meant to detect that is
> just fundamentally inadequate.  It's examining individual LOCALLOCK
> entries to detect conflicts, but we keep separate LOCALLOCK entries
> for different lockmodes on the same object.  So pg_advisory_lock()
> creates an entry with lockmode ExclusiveLock, while
> pg_advisory_xact_lock_shared() creates a different entry with lockmode
> AccessShareLock.  But they are pointing at the same PROCLOCK, and the
> restriction we're dealing with here applies at the PROCLOCK level.

For the archives: this has been applied as of 6310809.
--
Michael