Hot Standby WAL reply uses heavyweight session locks, but doesn't have enough infrastructure set up - Mailing list pgsql-hackers

From Andres Freund
Subject Hot Standby WAL reply uses heavyweight session locks, but doesn't have enough infrastructure set up
Date
Msg-id 20150126212458.GA29457@awork2.anarazel.de
Whole thread Raw
Responses Re: Hot Standby WAL reply uses heavyweight session locks, but doesn't have enough infrastructure set up
List pgsql-hackers
Hi,

dbase_redo does:    if (InHotStandby)    {        /*         * Lock database while we resolve conflicts to ensure that
      * InitPostgres() cannot fully re-execute concurrently. This         * avoids backends re-connecting automatically
tosame database,         * which can happen in some cases.         */
LockSharedObjectForSession(DatabaseRelationId,xlrec->db_id, 0, AccessExclusiveLock);
ResolveRecoveryConflictWithDatabase(xlrec->db_id);   }
 

Unfortunately that Assert()s when there's a lock conflict because
e.g. another backend is currently connecting. That's because ProcSleep()
does a enable_timeout_after(DEADLOCK_TIMEOUT, DeadlockTimeout) - and
there's no deadlock timeout (or lock timeout) handler registered.

I'm not sure if this is broken since 8bfd1a884 introducing those session
locks, or if it's caused by the new timeout infrastructure
(f34c68f09f34c68f09).

I guess the easiest way to fix this would be to make this a loop like
ResolveRecoveryConfictWithLock():


LOCKTAG        tag;

SET_LOCKTAG_OBJECT(tag,   InvalidOid,   DatabaseRelationId,   xlrec->db_id,   objsubid);

while (!lock_acquired)
{   while (CountDBBackends(dbid) > 0)   {       CancelDBBackends(dbid, PROCSIG_RECOVERY_CONFLICT_DATABASE, true);
       /*        * Wait awhile for them to die so that we avoid flooding an        * unresponsive backend when system
isheavily loaded.        */       pg_usleep(10000);   }
 

   if (LockAcquireExtended(&locktag, AccessExclusiveLock, true, true, false)           != LOCKACQUIRE_NOT_AVAIL)
  lock_acquired = true;
 
}

afaics, that should work? Not pretty, but probably easier than starting
to reason about the deadlock detector in the startup process.

We probably should also add a Assert(!InRecovery || sessionLock) to
LockAcquireExtended() - these kind of problems are otherwise hard to
find in a developer setting.

Greetings,

Andres Freund

--Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services



pgsql-hackers by date:

Previous
From: Jim Nasby
Date:
Subject: Re: pgaudit - an auditing extension for PostgreSQL
Next
From: Robert Haas
Date:
Subject: Re: New CF app deployment