> 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.
