Re: pg_advisory_locks in a multithreaded application context

On Thu, Jun 30, 2011 at 3:30 AM, Vincent Ficet
<jean-vincent(dot)ficet(at)bull(dot)net> wrote:
> Merlin Moncure wrote:
>> On Wed, Jun 29, 2011 at 10:11 AM, Vincent Ficet
>> <jean-vincent(dot)ficet(at)bull(dot)net> wrote:
>>
>>> Hello,
>>>
>>> I'm having some trouble trying to use postgresql locks. After trying
>>> several options and reading the postgresql online documentation, I still
>>> can't figure out what I'm doing wrong. Here's the use case:
>>>
>>> A multithreaded application collecting adapter firmwares on a network
>>> loads data into the following table:
>>>
>>> CREATE TABLE firmware (
>>>    id SERIAL NOT NULL,
>>>    type CHARACTER VARYING(32),
>>>    version CHARACTER VARYING(30),
>>>    build_id INTEGER,
>>>    date CHARACTER VARYING(25),
>>>    ps_id CHARACTER VARYING(25)
>>> );
>>>
>>> Typically, there are a few hundred adapters, but only 5 firmwares (many
>>> adapters should have the same firmware if the sysadmins did their jobs
>>> properly ;-) ).
>>>
>>> Only a single entry is required per firmware (many separate adapters can
>>> share the same firmware by pointing to the appropriate firmware id field).
>>> To make sure that only one entry is created per firmware, I use the
>>> following trigger:
>>>
>>> CREATE TRIGGER firmware_pre_insert_trigger
>>>    BEFORE INSERT ON firmware
>>>    FOR EACH ROW
>>>    EXECUTE PROCEDURE firmware_pre_insert_trigger_cb();
>>>
>>> CREATE FUNCTION firmware_pre_insert_trigger_cb() RETURNS TRIGGER
>>>    AS $_$
>>> DECLARE
>>>    fw_id INT;
>>> BEGIN
>>>
>>> SELECT fw.id FROM firmware fw INTO fw_id
>>> WHERE (fw.type = new.type AND
>>>       fw.version = new.version AND
>>>       fw.build_id = new.build_id AND
>>>       fw.date = new.date AND
>>>       fw.ps_id = new.ps_id);
>>>
>>> IF fw_id IS NULL THEN
>>>   -- create the non-existing firmware
>>>   RETURN new;
>>> ELSE
>>>   -- skip firmware which already exists
>>>   RETURN NULL;
>>> END IF;
>>>
>>> END;
>>> $_$
>>>    LANGUAGE PLPGSQL;
>>>
>>> When a thread wishes to add a firmware after discovering one adapter, it
>>> executes the following code:
>>>
>>> PERFORM pg_advisory_lock(1);
>>>
>>> INSERT INTO firmware (type, version, build_id, date, ps_id)
>>> VALUES (chip_type, firm_version, firm_build_id, firm_date, firm_ps_id);
>>>
>>> PERFORM pg_advisory_unlock(1);
>>>
>>
>> Advisory lock is not going to work here.  You are releasing the lock
>> before the transaction resolves and that leaves a window for  second
>> transaction to do the 'select' and not see the data because it hasn't
>> committed yet.
>>
>>
>>> Unfortunately, I still get duplicated entries using advisory locks, and
>>> they don't seem to lock anything at all...
>>>
>>> On the other hand, If I use builtin locks as follows without the
>>> trigger, I get deadlocks in the server logs:
>>>
>>> LOCK TABLE firmware IN SHARE MODE;
>>>
>>
>> well a sharelock certainly isn't going to work.  share blocks row
>> exclusive (see http://www.postgresql.org/docs/current/static/explicit-locking.html#LOCKING-TABLES),
>> so two transactions can simultaneously get a share lock and wait for
>> each other to to resolve to get the exclusive lock on a row.
>> 'EXCLUSIVE' would be better (although that would effectively serialize
>> the transactions).
>>
>>
> Thanks for the tip.
>
> It now works fine using a SHARE UPDATE EXCLUSIVE lock in the PRE INSERT
> trigger. This does not conflict with the  ROW EXCLUSIVE lock which is
> implicitely taken by the INSERT statement in add_firmware() function.
>
> BTW I think there might be a thread safety issue in postgres, as I often
> get a segfault when deadlocks occur (prior to applying the fix I just
> described). For example, the following deadlock situation:
>
>    DETAIL:  Process 7643 waits for RowExclusiveLock on relation 21060
> of database 20535; blocked by process 7593.
>            Process 7593 waits for RowExclusiveLock on relation 21060 of
> database 20535; blocked by process 7643.
>            Process 7643: SELECT
> add_firmware('0x08003800013731aa','hca','512.1792.0',0,'0920-10-06','BL_0010030001000');
>            Process 7593: SELECT
> add_firmware('0x08003800013734b0','hca','512.1792.0',0,'0920-10-06','BL_0010030001000');
>    HINT:  See server log for query details.
>    QUERY:  INSERT INTO firmware (type, version, build_id, date, ps_id)
>               VALUES (chip_type, firm_version, firm_build_id,
> firm_date, firm_ps_id)
>    CONTEXT:  PL/pgSQL function "add_firmware" line 31 at SQL statement
>
> Triggers:
>
> #0  0x0000003c46725742 in __strncpy_ssse3 () from /lib64/libc.so.6
> #1  0x00007ffff5f1617f in pqParseInput3 (conn=0x7ffff0000da0) at
> fe-protocol3.c:209
> #2  0x00007ffff5f0cae7 in parseInput (conn=0x7ffff0000da0) at fe-exec.c:1493
> #3  0x00007ffff5f0cc01 in PQgetResult (conn=0x7ffff0000da0) at
> fe-exec.c:1568
> #4  0x00007ffff5f0d26b in PQexecFinish (conn=0x7ffff0000da0) at
> fe-exec.c:1807
> #5  0x00007ffff5f0cee4 in PQexec (conn=0x7ffff0000da0,
> query=0x7ffff612d340 "SELECT 1") at fe-exec.c:1648
> #6  0x00007ffff612c344 in dbd_ping () from
> /home/vficet/X86_64/usr/lib/dbd/libdbdpgsql.so
>
> Stack traces often occur in different flavours, but can always be
> correlated with messages such as:
>
> DBI error -9: unexpected field count in "D" message
> DBI error -9: message contents do not agree with length in message type
> "T" server sent data ("D" message) without prior row description ("T"
> message)
>
> I'll try to set up a minimalist reproducer and see what can be done for
> that.
> Note that this happens with both postgresql 8.4.7 and 9.0.4.

is the segfault happening in the client? it looks like you have a
thread safety issue either in dbd or in your code...

merlin