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

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

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

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.

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

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.

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