RE: Questionable coding in proc.c & lock.c - Mailing list pgsql-hackers

From Hiroshi Inoue
Subject RE: Questionable coding in proc.c & lock.c
Date
Msg-id 000201bff857$c9908c80$2801007e@tpf.co.jp
Whole thread Raw
In response to Questionable coding in proc.c & lock.c  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
> -----Original Message-----
> From: pgsql-hackers-owner@hub.org [mailto:pgsql-hackers-owner@hub.org]On
> Behalf Of Tom Lane
> 
> 2. The loop in ProcLockWakeup() has been broken since the day it was
> written.  It's trying to scan through the list of waiting processes
> and awaken all that can legitimately be awoken.  However, as soon as
> it finds one that cannot be woken, it sets last_locktype = proc->token
> and then continues the loop *without advancing proc*.  All subsequent
> iterations will compare proc->token == last_locktype, find they are

I pointed out it once before 6.5 and it was one of the cause of lock
freezing then. I've known it has not been changed but neglected to tell it,
sorry. However the freezing was resolved by Vadim's change and I've
thought it doesn't cause freezing. Hmm,I don't remember well now how
the freezing occured.

The following is my posting about the freezing.
Hope that helps,

Regards.

Hiroshi Inoue
Inoue@tpf.co.jp

> There is no row-level locks: all locks over tables are
> table-level ones, btree & hash use page-level locks, but
> never do page->table level lock escalation.
>
> However, I'm not sure that proposed changes will help in the next case:
>
> session-1 => begin;
> session-1 => insert into tt values (1);    --RowExclusiveLock
>
> session-2 => begin;
> session-2 => insert into tt values (2);    --RowExclusiveLock
>
> session-3 => begin;
> session-3 => lock table tt;            --AccessExclusiveLock
>                 (conflicts with 1 & 2)
>                                 ^
> session-1 => lock table tt in share mode;    --ShareLock
>                 (conflicts with 2 & 3)
>                                     ^
> This is deadlock situation and must be handled by
> DeadLockCheck().
>

It's really a deadlock ?
Certainly end/abort of session-2 doesn't wakeup session-1/session3.
I think it's due to the following code in ProcLockWakeup().
      while ((queue_size--) && (proc))       {
               /*                * This proc will conflict as the previous one did, don't
even                * try.                */               if (proc->token == last_locktype)
continue;
               /*                * This proc conflicts with locks held by others, ignored.                */
  if (LockResolveConflicts(lockmethod,                                                                lock,
 

proc->token,                                                                proc->xid,

(XIDLookupEnt *
) NULL) != STATUS_OK)               {                       last_locktype = proc->token;
continue;              }
 

Once LockResolveConflicts() doesn't return STATUS_OK,proc
is not changed and only queue_size-- is executed(never try
to wakeup other procs).

After inserting the code such asproc = (PROC *) MAKE_PTR(proc->links.prev);
before continue statements,ProcLockWakeup() triggerd
by end/abort of session-2 could try to wakeup session-1.


pgsql-hackers by date:

Previous
From: Jeffery Collins
Date:
Subject: Re: Re: [GENERAL] Some questions on user defined types and functions.
Next
From: "Hiroshi Inoue"
Date:
Subject: RE: Questionable coding in proc.c & lock.c