lock mode for ControlFileLock which pg_start_backup uses
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
Attachments:
lockmode_v1.patchtext/x-patch; charset=US-ASCII; name=lockmode_v1.patchDownload
*** 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);
Fujii Masao <masao.fujii@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
Takahiro Itagaki wrote:
Fujii Masao <masao.fujii@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
Fujii Masao <masao.fujii@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