Re: [HACKERS] lock mode for ControlFileLock which pg_start_backup uses

2010-03-09 Thread Heikki Linnakangas 
Takahiro Itagaki wrote:
 Fujii Masao masao.fu...@gmail.com wrote:
 The attached patch changes the lock mode which pg_start_backup()
 uses. Is it worth applying this patch?
 
 I think the patch is reasonable to represent what we are doing,
 even if there is no performance benefits from it.

Agreed.

-- 
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] lock mode for ControlFileLock which pg_start_backup uses

2010-03-09 Thread Takahiro Itagaki 

Fujii Masao masao.fu...@gmail.com wrote:

 Currently pg_start_backup() accesses the shared ControlFile
 by using ControlFileLock with LW_EXCLUSIVE lock mode. But
 since that access is read-only operation, LW_SHARED should
 be chosen instead of LW_EXCLUSIVE.
 
 The attached patch changes the lock mode which pg_start_backup()
 uses. Is it worth applying this patch?

Thanks, applied.

Regards,
---
Takahiro Itagaki
NTT Open Source Software Center



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] lock mode for ControlFileLock which pg_start_backup uses

2010-03-08 Thread Fujii Masao 
Hi,

Currently pg_start_backup() accesses the shared ControlFile
by using ControlFileLock with LW_EXCLUSIVE lock mode. But
since that access is read-only operation, LW_SHARED should
be chosen instead of LW_EXCLUSIVE.

The attached patch changes the lock mode which pg_start_backup()
uses. Is it worth applying this patch?

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
*** a/src/backend/access/transam/xlog.c
--- b/src/backend/access/transam/xlog.c
***
*** 7897,7903  pg_start_backup(PG_FUNCTION_ARGS)
  		 * REDO pointer.  The oldest point in WAL that would be needed to
  		 * restore starting from the checkpoint is precisely the REDO pointer.
  		 */
! 		LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
  		checkpointloc = ControlFile-checkPoint;
  		startpoint = ControlFile-checkPointCopy.redo;
  		LWLockRelease(ControlFileLock);
--- 7897,7903 
  		 * REDO pointer.  The oldest point in WAL that would be needed to
  		 * restore starting from the checkpoint is precisely the REDO pointer.
  		 */
! 		LWLockAcquire(ControlFileLock, LW_SHARED);
  		checkpointloc = ControlFile-checkPoint;
  		startpoint = ControlFile-checkPointCopy.redo;
  		LWLockRelease(ControlFileLock);

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] lock mode for ControlFileLock which pg_start_backup uses

2010-03-08 Thread Takahiro Itagaki 

Fujii Masao masao.fu...@gmail.com wrote:

 Currently pg_start_backup() accesses the shared ControlFile
 by using ControlFileLock with LW_EXCLUSIVE lock mode. But
 since that access is read-only operation, LW_SHARED should
 be chosen instead of LW_EXCLUSIVE.

Almost all operations of ControlFileLock is in LW_EXCLUSIVE, but
there is one usage of LWLockConditionalAcquire(ControlFileLock, LW_SHARED)
in XLogNeedsFlush().

 The attached patch changes the lock mode which pg_start_backup()
 uses. Is it worth applying this patch?

I think the patch is reasonable to represent what we are doing,
even if there is no performance benefits from it.

Regards,
---
Takahiro Itagaki
NTT Open Source Software Center



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers