On Wed, Dec 23, 2015 at 9:40 PM, Jeff Janes <jeff.ja...@gmail.com> wrote:
> On Wed, Sep 23, 2015 at 11:33 PM, Jeff Janes <jeff.ja...@gmail.com> wrote:
>>
>> On further thought, neither do I.  The attached patch inverts
>> ResolveRecoveryConflictWithLock to be called back from the lmgr code so that
>> is it like ResolveRecoveryConflictWithBufferPin code.  It does not try to
>> cancel the conflicting lock holders from the signal handler, rather it just
>> loops an extra time and cancels the transactions on the next call.
>>
>> It looks like the deadlock detection is adequately handled within normal
>> lmgr code within the back-ends of the other parties to the deadlock, so I
>> didn't do a timeout for deadlock detection purposes.
>
> I was testing that this still applies to git HEAD.  And it doesn't due
> to the re-factoring of lock.h into lockdef.h.  (And apparently it
> never actually did, because that refactoring happened long before I
> wrote this patch.  I guess I must have done this work against
> 9_5_STABLE.)
>
> standby.h cannot include lock.h because standby.h is included
> indirectly by pg_xlogdump.  But it has to get LOCKTAG which is only in
> lock.h.
>
> Does this mean that standby.h also needs to get parts spun off into a
> new standbydef.h that can be included from front-end code?


That is how I've done it.

The lock cancel patch applies over the header split patch.

Cheers,

Jeff
diff --git a/src/backend/access/rmgrdesc/standbydesc.c b/src/backend/access/rmgrdesc/standbydesc.c
new file mode 100644
index 3d35f38..acd590b
*** a/src/backend/access/rmgrdesc/standbydesc.c
--- b/src/backend/access/rmgrdesc/standbydesc.c
***************
*** 14,20 ****
   */
  #include "postgres.h"
  
! #include "storage/standby.h"
  
  static void
  standby_desc_running_xacts(StringInfo buf, xl_running_xacts *xlrec)
--- 14,20 ----
   */
  #include "postgres.h"
  
! #include "storage/standbydefs.h"
  
  static void
  standby_desc_running_xacts(StringInfo buf, xl_running_xacts *xlrec)
diff --git a/src/bin/pg_xlogdump/rmgrdesc.c b/src/bin/pg_xlogdump/rmgrdesc.c
new file mode 100644
index 5b88a8d..f9cd395
*** a/src/bin/pg_xlogdump/rmgrdesc.c
--- b/src/bin/pg_xlogdump/rmgrdesc.c
***************
*** 27,33 ****
  #include "commands/tablespace.h"
  #include "replication/origin.h"
  #include "rmgrdesc.h"
! #include "storage/standby.h"
  #include "utils/relmapper.h"
  
  #define PG_RMGR(symname,name,redo,desc,identify,startup,cleanup) \
--- 27,33 ----
  #include "commands/tablespace.h"
  #include "replication/origin.h"
  #include "rmgrdesc.h"
! #include "storage/standbydefs.h"
  #include "utils/relmapper.h"
  
  #define PG_RMGR(symname,name,redo,desc,identify,startup,cleanup) \
diff --git a/src/include/storage/standby.h b/src/include/storage/standby.h
new file mode 100644
index 40b329b..f27a7ba
*** a/src/include/storage/standby.h
--- b/src/include/storage/standby.h
***************
*** 14,22 ****
  #ifndef STANDBY_H
  #define STANDBY_H
  
! #include "access/xlogreader.h"
! #include "lib/stringinfo.h"
! #include "storage/lockdefs.h"
  #include "storage/procsignal.h"
  #include "storage/relfilenode.h"
  
--- 14,20 ----
  #ifndef STANDBY_H
  #define STANDBY_H
  
! #include "storage/standbydefs.h"
  #include "storage/procsignal.h"
  #include "storage/relfilenode.h"
  
*************** extern void StandbyReleaseLockTree(Trans
*** 51,91 ****
  extern void StandbyReleaseAllLocks(void);
  extern void StandbyReleaseOldLocks(int nxids, TransactionId *xids);
  
- /*
-  * XLOG message types
-  */
- #define XLOG_STANDBY_LOCK			0x00
- #define XLOG_RUNNING_XACTS			0x10
- 
- typedef struct xl_standby_locks
- {
- 	int			nlocks;			/* number of entries in locks array */
- 	xl_standby_lock locks[FLEXIBLE_ARRAY_MEMBER];
- } xl_standby_locks;
- 
- /*
-  * When we write running xact data to WAL, we use this structure.
-  */
- typedef struct xl_running_xacts
- {
- 	int			xcnt;			/* # of xact ids in xids[] */
- 	int			subxcnt;		/* # of subxact ids in xids[] */
- 	bool		subxid_overflow;	/* snapshot overflowed, subxids missing */
- 	TransactionId nextXid;		/* copy of ShmemVariableCache->nextXid */
- 	TransactionId oldestRunningXid;		/* *not* oldestXmin */
- 	TransactionId latestCompletedXid;	/* so we can set xmax */
- 
- 	TransactionId xids[FLEXIBLE_ARRAY_MEMBER];
- } xl_running_xacts;
- 
  #define MinSizeOfXactRunningXacts offsetof(xl_running_xacts, xids)
  
  
- /* Recovery handlers for the Standby Rmgr (RM_STANDBY_ID) */
- extern void standby_redo(XLogReaderState *record);
- extern void standby_desc(StringInfo buf, XLogReaderState *record);
- extern const char *standby_identify(uint8 info);
- 
  /*
   * Declarations for GetRunningTransactionData(). Similar to Snapshots, but
   * not quite. This has nothing at all to do with visibility on this server,
--- 49,57 ----
diff --git a/src/include/storage/standbydefs.h b/src/include/storage/standbydefs.h
new file mode 100644
index ...a2c9db8
*** a/src/include/storage/standbydefs.h
--- b/src/include/storage/standbydefs.h
***************
*** 0 ****
--- 1,53 ----
+ /*-------------------------------------------------------------------------
+  *
+  * standbydef.h
+  *	   Frontend exposed definitions for hot standby mode.
+  *
+  *
+  * Portions Copyright (c) 1996-2015, PostgreSQL Global Development Group
+  * Portions Copyright (c) 1994, Regents of the University of California
+  *
+  * src/include/storage/standbydefs.h
+  *
+  *-------------------------------------------------------------------------
+  */
+ #ifndef STANDBYDEFS_H
+ #define STANDBYDEFS_H
+ 
+ #include "access/xlogreader.h"
+ #include "lib/stringinfo.h"
+ #include "storage/lockdefs.h"
+ 
+ /* Recovery handlers for the Standby Rmgr (RM_STANDBY_ID) */
+ extern void standby_redo(XLogReaderState *record);
+ extern void standby_desc(StringInfo buf, XLogReaderState *record);
+ extern const char *standby_identify(uint8 info);
+ 
+ /*
+  * XLOG message types
+  */
+ #define XLOG_STANDBY_LOCK			0x00
+ #define XLOG_RUNNING_XACTS			0x10
+ 
+ typedef struct xl_standby_locks
+ {
+ 	int			nlocks;			/* number of entries in locks array */
+ 	xl_standby_lock locks[FLEXIBLE_ARRAY_MEMBER];
+ } xl_standby_locks;
+ 
+ /*
+  * When we write running xact data to WAL, we use this structure.
+  */
+ typedef struct xl_running_xacts
+ {
+ 	int			xcnt;			/* # of xact ids in xids[] */
+ 	int			subxcnt;		/* # of subxact ids in xids[] */
+ 	bool		subxid_overflow;	/* snapshot overflowed, subxids missing */
+ 	TransactionId nextXid;		/* copy of ShmemVariableCache->nextXid */
+ 	TransactionId oldestRunningXid;		/* *not* oldestXmin */
+ 	TransactionId latestCompletedXid;	/* so we can set xmax */
+ 
+ 	TransactionId xids[FLEXIBLE_ARRAY_MEMBER];
+ } xl_running_xacts;
+ 
+ #endif   /* STANDBYDEFS_H */
diff --git a/src/backend/postmaster/startup.c b/src/backend/postmaster/startup.c
new file mode 100644
index 581837a..b993ca8
*** a/src/backend/postmaster/startup.c
--- b/src/backend/postmaster/startup.c
*************** StartupProcessMain(void)
*** 203,208 ****
--- 203,209 ----
  	 */
  	RegisterTimeout(STANDBY_DEADLOCK_TIMEOUT, StandbyDeadLockHandler);
  	RegisterTimeout(STANDBY_TIMEOUT, StandbyTimeoutHandler);
+ 	RegisterTimeout(STANDBY_LOCK_TIMEOUT, StandbyLockTimeoutHandler);
  
  	/*
  	 * Unblock signals (they were blocked when the postmaster forked us)
diff --git a/src/backend/storage/ipc/standby.c b/src/backend/storage/ipc/standby.c
new file mode 100644
index 292bed5..4b428de
*** a/src/backend/storage/ipc/standby.c
--- b/src/backend/storage/ipc/standby.c
*************** static List *RecoveryLockList;
*** 41,47 ****
  
  static void ResolveRecoveryConflictWithVirtualXIDs(VirtualTransactionId *waitlist,
  									   ProcSignalReason reason);
- static void ResolveRecoveryConflictWithLock(Oid dbOid, Oid relOid);
  static void SendRecoveryConflictWithBufferPin(ProcSignalReason reason);
  static XLogRecPtr LogCurrentRunningXacts(RunningTransactions CurrRunningXacts);
  static void LogAccessExclusiveLocks(int nlocks, xl_standby_lock *locks);
--- 41,46 ----
*************** ResolveRecoveryConflictWithDatabase(Oid
*** 337,375 ****
  	}
  }
  
! static void
! ResolveRecoveryConflictWithLock(Oid dbOid, Oid relOid)
  {
! 	VirtualTransactionId *backends;
! 	bool		lock_acquired = false;
! 	int			num_attempts = 0;
! 	LOCKTAG		locktag;
  
! 	SET_LOCKTAG_RELATION(locktag, dbOid, relOid);
  
! 	/*
! 	 * If blowing away everybody with conflicting locks doesn't work, after
! 	 * the first two attempts then we just start blowing everybody away until
! 	 * it does work. We do this because its likely that we either have too
! 	 * many locks and we just can't get one at all, or that there are many
! 	 * people crowding for the same table. Recovery must win; the end
! 	 * justifies the means.
! 	 */
! 	while (!lock_acquired)
! 	{
! 		if (++num_attempts < 3)
! 			backends = GetLockConflicts(&locktag, AccessExclusiveLock);
! 		else
! 			backends = GetConflictingVirtualXIDs(InvalidTransactionId,
! 												 InvalidOid);
  
  		ResolveRecoveryConflictWithVirtualXIDs(backends,
  											 PROCSIG_RECOVERY_CONFLICT_LOCK);
  
! 		if (LockAcquireExtended(&locktag, AccessExclusiveLock, true, true, false)
! 			!= LOCKACQUIRE_NOT_AVAIL)
! 			lock_acquired = true;
  	}
  }
  
  /*
--- 336,402 ----
  	}
  }
  
! /*
!  * ResolveRecoveryConflictWithLock is called from ProcSleep()
!  * to resolve conflicts with other backends holding relation locks.
!  *
!  * The WaitLatch sleep normally done in ProcSleep()
!  * (when not InHotStandby) is performed here, for code clarity.
!  *
!  * We either resolve conflicts immediately or set a timeout to wake us at
!  * the limit of our patience.
!  *
!  * Resolve conflicts by cancelling to all backends holding a conflicting 
!  * lock.  As we are already queued to be granted the lock, no new lock
!  * requests conflicting with ours will be granted in the meantime.
!  *
!  * Deadlocks involving the Startup process and an ordinary backend process 
!  * will be detected by the deadlock detector within the ordinary backend.
!  */
! void
! ResolveRecoveryConflictWithLock(LOCKTAG locktag)
  {
! 	TimestampTz ltime;
  
! 	Assert(InHotStandby);
  
! 	ltime = GetStandbyLimitTime();
  
+ 	if (GetCurrentTimestamp() >= ltime)
+ 	{
+ 		/*
+ 		 * We're already behind, so clear a path as quickly as possible.
+ 		 */
+ 		VirtualTransactionId *backends;
+ 		backends = GetLockConflicts(&locktag, AccessExclusiveLock);
  		ResolveRecoveryConflictWithVirtualXIDs(backends,
  											 PROCSIG_RECOVERY_CONFLICT_LOCK);
+ 	}
+ 	else
+ 	{
+ 		/*
+ 		 * Wake up at ltime
+ 		 */
+ 		EnableTimeoutParams timeouts[1];
  
! 		timeouts[0].id = STANDBY_LOCK_TIMEOUT;
! 		timeouts[0].type = TMPARAM_AT;
! 		timeouts[0].fin_time = ltime;
! 		enable_timeouts(timeouts, 1);
  	}
+ 
+ 	/* Wait to be signaled by the release of the Relation Lock */
+ 	ProcWaitForSignal();
+ 
+ 	/*
+ 	 * Clear any timeout requests established above.  We assume here that the
+ 	 * Startup process doesn't have any other outstanding timeouts than what 
+ 	 * this function
+ 	 * uses.  If that stops being true, we could cancel the timeouts
+ 	 * individually, but that'd be slower.
+ 	 */
+ 	disable_all_timeouts(false);
+ 
  }
  
  /*
*************** StandbyTimeoutHandler(void)
*** 532,537 ****
--- 559,574 ----
  	SendRecoveryConflictWithBufferPin(PROCSIG_RECOVERY_CONFLICT_BUFFERPIN);
  }
  
+ /*
+  * StandbyLockTimeoutHandler() will be called if STANDBY_LOCK_TIMEOUT is exceeded.
+  * This doesn't need to do anything, simply waking up is enough.
+  */
+ void
+ StandbyLockTimeoutHandler(void)
+ {
+ 	/* forget any pending STANDBY_DEADLOCK_TIMEOUT request */
+ 	disable_timeout(STANDBY_DEADLOCK_TIMEOUT, false);
+ }
  
  /*
   * -----------------------------------------------------
*************** StandbyTimeoutHandler(void)
*** 545,551 ****
   * process is the proxy by which the original locks are implemented.
   *
   * We only keep track of AccessExclusiveLocks, which are only ever held by
!  * one transaction on one relation, and don't worry about lock queuing.
   *
   * We keep a single dynamically expandible list of locks in local memory,
   * RelationLockList, so we can keep track of the various entries made by
--- 582,588 ----
   * process is the proxy by which the original locks are implemented.
   *
   * We only keep track of AccessExclusiveLocks, which are only ever held by
!  * one transaction on one relation.
   *
   * We keep a single dynamically expandible list of locks in local memory,
   * RelationLockList, so we can keep track of the various entries made by
*************** StandbyAcquireAccessExclusiveLock(Transa
*** 587,600 ****
  	newlock->relOid = relOid;
  	RecoveryLockList = lappend(RecoveryLockList, newlock);
  
- 	/*
- 	 * Attempt to acquire the lock as requested, if not resolve conflict
- 	 */
  	SET_LOCKTAG_RELATION(locktag, newlock->dbOid, newlock->relOid);
  
! 	if (LockAcquireExtended(&locktag, AccessExclusiveLock, true, true, false)
! 		== LOCKACQUIRE_NOT_AVAIL)
! 		ResolveRecoveryConflictWithLock(newlock->dbOid, newlock->relOid);
  }
  
  static void
--- 624,632 ----
  	newlock->relOid = relOid;
  	RecoveryLockList = lappend(RecoveryLockList, newlock);
  
  	SET_LOCKTAG_RELATION(locktag, newlock->dbOid, newlock->relOid);
  
! 	LockAcquireExtended(&locktag, AccessExclusiveLock, true, false, false);
  }
  
  static void
diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c
new file mode 100644
index bb10c1b..4f13f9d
*** a/src/backend/storage/lmgr/proc.c
--- b/src/backend/storage/lmgr/proc.c
***************
*** 42,47 ****
--- 42,48 ----
  #include "postmaster/autovacuum.h"
  #include "replication/slot.h"
  #include "replication/syncrep.h"
+ #include "storage/standby.h"
  #include "storage/ipc.h"
  #include "storage/lmgr.h"
  #include "storage/pmsignal.h"
*************** ProcSleep(LOCALLOCK *locallock, LockMeth
*** 1080,1099 ****
  	 * If LockTimeout is set, also enable the timeout for that.  We can save a
  	 * few cycles by enabling both timeout sources in one call.
  	 */
! 	if (LockTimeout > 0)
  	{
! 		EnableTimeoutParams timeouts[2];
! 
! 		timeouts[0].id = DEADLOCK_TIMEOUT;
! 		timeouts[0].type = TMPARAM_AFTER;
! 		timeouts[0].delay_ms = DeadlockTimeout;
! 		timeouts[1].id = LOCK_TIMEOUT;
! 		timeouts[1].type = TMPARAM_AFTER;
! 		timeouts[1].delay_ms = LockTimeout;
! 		enable_timeouts(timeouts, 2);
  	}
- 	else
- 		enable_timeout_after(DEADLOCK_TIMEOUT, DeadlockTimeout);
  
  	/*
  	 * If somebody wakes us between LWLockRelease and WaitLatch, the latch
--- 1081,1103 ----
  	 * If LockTimeout is set, also enable the timeout for that.  We can save a
  	 * few cycles by enabling both timeout sources in one call.
  	 */
! 	if (!InHotStandby)
  	{
! 		if (LockTimeout > 0)
! 		{
! 			EnableTimeoutParams timeouts[2];
! 	
! 			timeouts[0].id = DEADLOCK_TIMEOUT;
! 			timeouts[0].type = TMPARAM_AFTER;
! 			timeouts[0].delay_ms = DeadlockTimeout;
! 			timeouts[1].id = LOCK_TIMEOUT;
! 			timeouts[1].type = TMPARAM_AFTER;
! 			timeouts[1].delay_ms = LockTimeout;
! 			enable_timeouts(timeouts, 2);
! 		}
! 		else
! 			enable_timeout_after(DEADLOCK_TIMEOUT, DeadlockTimeout);
  	}
  
  	/*
  	 * If somebody wakes us between LWLockRelease and WaitLatch, the latch
*************** ProcSleep(LOCALLOCK *locallock, LockMeth
*** 1111,1125 ****
  	 */
  	do
  	{
! 		WaitLatch(MyLatch, WL_LATCH_SET, 0);
! 		ResetLatch(MyLatch);
! 		/* check for deadlocks first, as that's probably log-worthy */
! 		if (got_deadlock_timeout)
  		{
! 			CheckDeadLock();
! 			got_deadlock_timeout = false;
  		}
- 		CHECK_FOR_INTERRUPTS();
  
  		/*
  		 * waitStatus could change from STATUS_WAITING to something else
--- 1115,1137 ----
  	 */
  	do
  	{
! 		if (InHotStandby)
  		{
! 			/* Set a timer and wait for that or for the Lock to be granted */
! 			ResolveRecoveryConflictWithLock(locallock->tag.lock);
! 		}
! 		else
! 		{
! 			WaitLatch(MyLatch, WL_LATCH_SET, 0);
! 			ResetLatch(MyLatch);
! 			/* check for deadlocks first, as that's probably log-worthy */
! 			if (got_deadlock_timeout)
! 			{
! 				CheckDeadLock();
! 				got_deadlock_timeout = false;
! 			}
! 			CHECK_FOR_INTERRUPTS();
  		}
  
  		/*
  		 * waitStatus could change from STATUS_WAITING to something else
diff --git a/src/include/storage/standby.h b/src/include/storage/standby.h
new file mode 100644
index f27a7ba..957ba19
*** a/src/include/storage/standby.h
--- b/src/include/storage/standby.h
***************
*** 15,20 ****
--- 15,21 ----
  #define STANDBY_H
  
  #include "storage/standbydefs.h"
+ #include "storage/lock.h"
  #include "storage/procsignal.h"
  #include "storage/relfilenode.h"
  
*************** extern void ResolveRecoveryConflictWithS
*** 31,40 ****
--- 32,43 ----
  extern void ResolveRecoveryConflictWithTablespace(Oid tsid);
  extern void ResolveRecoveryConflictWithDatabase(Oid dbid);
  
+ extern void ResolveRecoveryConflictWithLock(LOCKTAG locktag);
  extern void ResolveRecoveryConflictWithBufferPin(void);
  extern void CheckRecoveryConflictDeadlock(void);
  extern void StandbyDeadLockHandler(void);
  extern void StandbyTimeoutHandler(void);
+ extern void StandbyLockTimeoutHandler(void);
  
  /*
   * Standby Rmgr (RM_STANDBY_ID)
diff --git a/src/include/utils/timeout.h b/src/include/utils/timeout.h
new file mode 100644
index 30581a1..33b5016
*** a/src/include/utils/timeout.h
--- b/src/include/utils/timeout.h
*************** typedef enum TimeoutId
*** 29,34 ****
--- 29,35 ----
  	STATEMENT_TIMEOUT,
  	STANDBY_DEADLOCK_TIMEOUT,
  	STANDBY_TIMEOUT,
+ 	STANDBY_LOCK_TIMEOUT,
  	/* First user-definable timeout reason */
  	USER_TIMEOUT,
  	/* Maximum number of timeout reasons */
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to