Date: Fri, 19 Jun 2026 17:11:11 +0200
From: Dmitry Dolgov <9erthalion6@gmail.com>
To: Ilmar Yunusov <tanswis42@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: File locks for data directory lockfile in the context of Linux
 namespaces
Message-ID: <54uh7vhavffktp72dmjm7ms26lntxacnvpwuaitiez4izugdi6@emwc2yrpqwk2>
References: <z2u3jtjzhqofqhrjvfgkwl4cczhpqibz3t47gpx7fbe7ceaihk@i5mveu2wfpxn>
 <45swup77t7utfy33lu47mvkb4exzm3oeakz25abqxuvmsyzt3c@5umrpfgc3ypg>
 <178066304625.594057.12185296716401427152.pgcf@coridan.postgresql.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: 
 <178066304625.594057.12185296716401427152.pgcf@coridan.postgresql.org>
Archived-At: 
 <https://www.postgresql.org/message-id/54uh7vhavffktp72dmjm7ms26lntxacnvpwuaitiez4izugdi6%40emwc2yrpqwk2>
Precedence: bulk

> On Fri, Jun 05, 2026 at 12:37:26PM +0000, Ilmar Yunusov wrote:
> The following review has been posted through the commitfest application:
> make installcheck-world:  not tested
> Implements feature:       tested, passed
> Spec compliant:           not tested
> Documentation:            not tested
> 
> Hi,
> 
> I looked at v2, focusing on apply/build status and the PID namespace scenario
> described in the cover letter.

Thanks for the review and testing!

> Is locking the socket lock file intentional here? If so, maybe the helper name,
> comment, and fd tracking should reflect that broader scope. If not, perhaps the
> new lock should be applied only for the isDDLock case; otherwise v2 changes
> socket lockfile semantics too, not only postmaster.pid.

It's been a while, quite frankly I don't remember. On the face of it, I
think both the directory and socket lock files should be locked in the
same way, since both are equally susceptible for the failure scenario
described in this thread, even if the consequences of a failure are
different. Let me do some renaming to clarify that.

> While reading that code, I also noticed a small error-path issue: DataDirLockFD
> starts as 0, but if fcntl(F_OFD_SETLK) fails with a non-EAGAIN error the code
> only emits a warning and does not assign a duplicate fd. UnlinkLockFiles()
> then closes DataDirLockFD unconditionally. Initializing it to -1, closing it
> conditionally, and checking dup(fd) would make that path more explicit.

Good point, I'll address this in the next version.