Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> writes:
>> Hmm, I think it would be simpler to decide that instead of
>> SerializableXactHashLock, you must hold ProcArrayLock to access
>> LastSxactCommitSeqNo, and move the assignment of commitSeqNo to
>> ProcArrayTransaction(). It's probably easiest to move
>> LastSxactCommitSeqno to ShmemVariableCache too. There's a few
>> places that would then need to acquire ProcArrayLock to read
>> LastSxactCommitSeqno, but I feel it might still be much simpler
>> that way.
>
> Yeah ... this patch creats the need to hold both
> SerializableXactHashLock and ProcArrayLock during transaction
> commit, which is a bit scary from a deadlock-risk perspective,
If I remember correctly, we already do some calls to functions which
acquire ProcArrayLock while holding SerializableXactHashLock, in
order to acquire a snapshot with the right timing. Regardless of
what we do on transaction completion, we either need to document
that in the code comments, or do some major surgery at the last
minute, which is always scary.
> and not pleasant from the concurrency standpoint either.
On the bright side, both locks are not held at the same time unless
the transaction isolation level is serializable.
> It'd be better to push some functionality into the procarray code.
That's easily done if we don't mind taking out a ProcArrayLock
during completion of a transaction which has no XID, if only long
enough to increment a uint64 in shared memory, and then stash the
value -- somewhere -- so that SSI code can find and use it. We
thought it was best to avoid that so there wasn't any impact on
non-serializable transactions.
-Kevin