Обсуждение: SHARED locks barging behaviour

On Tue, Jan 17, 2023 at 12:18:28PM -0500, Arul Ajmani wrote:
> I'm trying to better understand the following barging behaviour with SHARED
> locks.
...
> Given there is a transaction waiting to acquire a FOR UPDATE lock, I was
> surprised to see the second FOR SHARE transaction return immediately instead of
> waiting. I have two questions:
> 
> 1) Could this barging behaviour potentially starve out the transaction waiting
> to acquire the FOR UPDATE lock, if there is a continuous queue of transactions
> that acquire a FOR SHARE lock briefly?

Yes, see below.

> 2) Assuming this is by design, I couldn't find (in code) where this explicit
> policy choice is made. I was looking around LockAcquireExtended, but it seems
> like the decision is made above this layer. Could someone more familiar with
> this code point me at the right place? 

I know this from January, but I do have an answer.  First, looking at
parser/gram.y, I see:

            | FOR SHARE                         { $$ = LCS_FORSHARE; }

Looking for LCS_FORSHARE, I see in optimizer/plan/planner.c:

            case LCS_FORSHARE:
                return ROW_MARK_SHARE;

Looking for ROW_MARK_SHARE, I see in executor/nodeLockRows.c:

            case ROW_MARK_SHARE:
                lockmode = LockTupleShare;

Looking for LockTupleShare, I see in access/heap/heapam.c:

        else if (mode == LockTupleShare)
        {
            /*
             * If we're requesting Share, we can similarly avoid sleeping if
             * there's no update and no exclusive lock present.
             */
            if (HEAP_XMAX_IS_LOCKED_ONLY(infomask) &&
                !HEAP_XMAX_IS_EXCL_LOCKED(infomask))
            {
                LockBuffer(*buffer, BUFFER_LOCK_EXCLUSIVE);

                /*
                 * Make sure it's still an appropriate lock, else start over.
                 * See above about allowing xmax to change.
                 */
                if (!HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_data->t_infomask) ||
                    HEAP_XMAX_IS_EXCL_LOCKED(tuple->t_data->t_infomask))
                    goto l3;
                require_sleep = false;
            }
        }

and this is basically saying that if the row is locked
(HEAP_XMAX_IS_LOCKED_ONLY), but not exclusively locked
(!HEAP_XMAX_IS_EXCL_LOCKED), then there is no need to sleep waiting for
the lock.

I hope that helps.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Only you can decide what is important to you.

On Fri, 2023-09-29 at 17:45 -0400, Bruce Momjian wrote:
> On Tue, Jan 17, 2023 at 12:18:28PM -0500, Arul Ajmani wrote:
> > I'm trying to better understand the following barging behaviour with SHARED
> > locks.
> ...
> > Given there is a transaction waiting to acquire a FOR UPDATE lock, I was
> > surprised to see the second FOR SHARE transaction return immediately instead of
> > waiting. I have two questions:
> >
> > 1) Could this barging behaviour potentially starve out the transaction waiting
> > to acquire the FOR UPDATE lock, if there is a continuous queue of transactions
> > that acquire a FOR SHARE lock briefly?
>
> Yes, see below.
>
> > 2) Assuming this is by design, I couldn't find (in code) where this explicit
> > policy choice is made. I was looking around LockAcquireExtended, but it seems
> > like the decision is made above this layer. Could someone more familiar with
> > this code point me at the right place? 
>
> I know this from January, but I do have an answer.  [...]

You answer the question where this is implemented.  But the more important question
is whether this is intentional.  This code was added by 0ac5ad5134f (introducing
FOR KEY SHARE and FOR NO KEY UPDATE).  My feeling is that it is not intentional that
a continuous stream of share row locks can starve out an exclusive row lock, since
PostgreSQL behaves differently with other locks.

On the other hand, if nobody has complained about it in these ten years, perhaps
it is just fine the way it is, if by design or not.

Yours,
Laurenz Albe

Hi,

On 2023-09-30 00:50:11 +0200, Laurenz Albe wrote:
> On Fri, 2023-09-29 at 17:45 -0400, Bruce Momjian wrote:
> > On Tue, Jan 17, 2023 at 12:18:28PM -0500, Arul Ajmani wrote:
> > > I'm trying to better understand the following barging behaviour with SHARED
> > > locks.
> > ...
> > > Given there is a transaction waiting to acquire a FOR UPDATE lock, I was
> > > surprised to see the second FOR SHARE transaction return immediately instead of
> > > waiting. I have two questions:
> > > 
> > > 1) Could this barging behaviour potentially starve out the transaction waiting
> > > to acquire the FOR UPDATE lock, if there is a continuous queue of transactions
> > > that acquire a FOR SHARE lock briefly?
> > 
> > Yes, see below.
> > 
> > > 2) Assuming this is by design, I couldn't find (in code) where this explicit
> > > policy choice is made. I was looking around LockAcquireExtended, but it seems
> > > like the decision is made above this layer. Could someone more familiar with
> > > this code point me at the right place? 
> > 
> > I know this from January, but I do have an answer.  [...]
> 
> You answer the question where this is implemented.  But the more important question
> is whether this is intentional.  This code was added by 0ac5ad5134f (introducing
> FOR KEY SHARE and FOR NO KEY UPDATE).  My feeling is that it is not intentional that
> a continuous stream of share row locks can starve out an exclusive row lock, since
> PostgreSQL behaves differently with other locks.
> 
> On the other hand, if nobody has complained about it in these ten years, perhaps
> it is just fine the way it is, if by design or not.

I'd be very hesitant to change the behaviour at this point - the likelihood of
existing workloads slowing down substantially, or even breaking due to an
additional source of deadlocks, seems substantial.

Greetings,

Andres Freund