Обсуждение: 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

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