Обсуждение: Error in backend/storage/lmgr/proc.c: ProcSleep()

Поиск
Список
Период
Сортировка

Error in backend/storage/lmgr/proc.c: ProcSleep()

От
Tomasz Zielonka
Дата:
Hi!

Platform: PostgreSQL 7.1.3, Linux 2.4.8, egcs 2.91.66

PostgreSQL forgets to release lock until shutdown in this scenario:

CREATE TABLE dummy (X integer);


session1                       session2
------------------------------------------------------------
BEGIN;                       |
                             |BEGIN;
SELECT * FROM dummy;         |
                             |SELECT * FROM dummy;
LOCK TABLE dummy;            |
                             |LOCK TABLE dummy;
                              deadlock....
\q                            \q

Deadlock is OK here, but I can't access dummy table until I restart postmaster

Everything's fine, when I comment out * marked lines in
backend/storage/lmgr/proc.c:

      if (myHeldLocks != 0)
      {
          int         aheadRequests = 0;

          proc = (PROC *) MAKE_PTR(waitQueue->links.next);
          for (i = 0; i < waitQueue->size; i++)
          {
              /* Must he wait for me? */
              if (lockctl->conflictTab[proc->waitLockMode] & myHeldLocks)
              {
                  /* Must I wait for him ? */
*                 if (lockctl->conflictTab[lockmode] & proc->heldLocks)
*                 {
*                     /* Yes, can report deadlock failure immediately */
*                     MyProc->errType = STATUS_ERROR;
*                     return STATUS_ERROR;
*                 }

Then the standard deadlock detection procedure is used.

greetings,
tom

--
.signature: Too many levels of symbolic links

Re: Error in backend/storage/lmgr/proc.c: ProcSleep()

От
Tom Lane
Дата:
Tomasz Zielonka <tomek@mult.i.pl> writes:
> Platform: PostgreSQL 7.1.3, Linux 2.4.8, egcs 2.91.66
> PostgreSQL forgets to release lock until shutdown in this scenario:

This seems to be a real bug, but I don't like your proposed fix...
the problem evidently is that some lock structure is not being cleaned
up, and I don't see how this change relates to that.

            regards, tom lane

Re: Error in backend/storage/lmgr/proc.c: ProcSleep()

От
Tom Lane
Дата:
Tomasz Zielonka <tomek@mult.i.pl> writes:
> Platform: PostgreSQL 7.1.3, Linux 2.4.8, egcs 2.91.66
> PostgreSQL forgets to release lock until shutdown in this scenario:

Good catch!  This has been broken since 7.1 ... surprising that no one
discovered the problem sooner.

I think that rather than removing the early-deadlock-detection code as
you suggest, it's better to make it work correctly.  I have applied a
patch that allows RemoveFromWaitQueue() to be used, so that the recovery
path is the same as if HandleDeadLock had been invoked.

            regards, tom lane


*** /home/postgres/pgsql/src/backend/storage/lmgr/proc.c.orig    Fri Jul  6 17:04:26 2001
--- /home/postgres/pgsql/src/backend/storage/lmgr/proc.c    Mon Sep  3 22:26:57 2001
***************
*** 506,521 ****
      SPINLOCK    spinlock = lockctl->masterLock;
      PROC_QUEUE *waitQueue = &(lock->waitProcs);
      int            myHeldLocks = MyProc->heldLocks;
      PROC       *proc;
      int            i;
-
  #ifndef __BEOS__
      struct itimerval timeval,
                  dummy;
-
  #else
      bigtime_t    time_interval;
-
  #endif

      /*
--- 506,519 ----
      SPINLOCK    spinlock = lockctl->masterLock;
      PROC_QUEUE *waitQueue = &(lock->waitProcs);
      int            myHeldLocks = MyProc->heldLocks;
+     bool        early_deadlock = false;
      PROC       *proc;
      int            i;
  #ifndef __BEOS__
      struct itimerval timeval,
                  dummy;
  #else
      bigtime_t    time_interval;
  #endif

      /*
***************
*** 535,541 ****
       * immediately.  This is the same as the test for immediate grant in
       * LockAcquire, except we are only considering the part of the wait
       * queue before my insertion point.
-      *
       */
      if (myHeldLocks != 0)
      {
--- 533,538 ----
***************
*** 550,558 ****
                  /* Must I wait for him ? */
                  if (lockctl->conflictTab[lockmode] & proc->heldLocks)
                  {
!                     /* Yes, can report deadlock failure immediately */
!                     MyProc->errType = STATUS_ERROR;
!                     return STATUS_ERROR;
                  }
                  /* I must go before this waiter.  Check special case. */
                  if ((lockctl->conflictTab[lockmode] & aheadRequests) == 0 &&
--- 547,560 ----
                  /* Must I wait for him ? */
                  if (lockctl->conflictTab[lockmode] & proc->heldLocks)
                  {
!                     /*
!                      * Yes, so we have a deadlock.  Easiest way to clean up
!                      * correctly is to call RemoveFromWaitQueue(), but we
!                      * can't do that until we are *on* the wait queue.
!                      * So, set a flag to check below, and break out of loop.
!                      */
!                     early_deadlock = true;
!                     break;
                  }
                  /* I must go before this waiter.  Check special case. */
                  if ((lockctl->conflictTab[lockmode] & aheadRequests) == 0 &&
***************
*** 600,606 ****
      MyProc->waitHolder = holder;
      MyProc->waitLockMode = lockmode;

!     MyProc->errType = STATUS_OK;/* initialize result for success */

      /* mark that we are waiting for a lock */
      waitingForLock = true;
--- 602,620 ----
      MyProc->waitHolder = holder;
      MyProc->waitLockMode = lockmode;

!     MyProc->errType = STATUS_OK; /* initialize result for success */
!
!     /*
!      * If we detected deadlock, give up without waiting.  This must agree
!      * with HandleDeadLock's recovery code, except that we shouldn't release
!      * the semaphore since we haven't tried to lock it yet.
!      */
!     if (early_deadlock)
!     {
!         RemoveFromWaitQueue(MyProc);
!         MyProc->errType = STATUS_ERROR;
!         return STATUS_ERROR;
!     }

      /* mark that we are waiting for a lock */
      waitingForLock = true;

Re: Error in backend/storage/lmgr/proc.c: ProcSleep()

От
Tomasz Zielonka
Дата:
On Mon, Sep 03, 2001 at 07:56:08PM -0400, Tom Lane wrote:
> Tomasz Zielonka <tomek@mult.i.pl> writes:
> > Platform: PostgreSQL 7.1.3, Linux 2.4.8, egcs 2.91.66
> > PostgreSQL forgets to release lock until shutdown in this scenario:
>
> This seems to be a real bug, but I don't like your proposed fix...

It was not supposed to be 'proposed fix', just a proposition where the
bug could be.

thanks,
Tom

--
.signature: Too many levels of symbolic links