public inbox for [email protected]  
help / color / mirror / Atom feed
Re: Disallow altering invalidated replication slots
3+ messages / 3 participants
[nested] [flat]

* Re: Disallow altering invalidated replication slots
@ 2024-09-11 03:11  shveta malik <[email protected]>
  0 siblings, 1 reply; 3+ messages in thread

From: shveta malik @ 2024-09-11 03:11 UTC (permalink / raw)
  To: Bharath Rupireddy <[email protected]>; +Cc: Peter Smith <[email protected]>; PostgreSQL Hackers <[email protected]>; shveta malik <[email protected]>

On Tue, Sep 10, 2024 at 11:24 PM Bharath Rupireddy
<[email protected]> wrote:
>
>
> Please find the attached v2 patch also having Shveta's review comments
> addressed.

The v2 patch looks good to me.

thanks
Shveta






^ permalink  raw  reply  [nested|flat] 3+ messages in thread

* Re: Disallow altering invalidated replication slots
@ 2024-09-12 10:54  Amit Kapila <[email protected]>
  parent: shveta malik <[email protected]>
  0 siblings, 0 replies; 3+ messages in thread

From: Amit Kapila @ 2024-09-12 10:54 UTC (permalink / raw)
  To: shveta malik <[email protected]>; +Cc: Bharath Rupireddy <[email protected]>; Peter Smith <[email protected]>; PostgreSQL Hackers <[email protected]>

On Wed, Sep 11, 2024 at 8:41 AM shveta malik <[email protected]> wrote:
>
> On Tue, Sep 10, 2024 at 11:24 PM Bharath Rupireddy
> <[email protected]> wrote:
> >
> >
> > Please find the attached v2 patch also having Shveta's review comments
> > addressed.
>
> The v2 patch looks good to me.
>

LGTM as well. I'll push this tomorrow morning unless there are more
comments or suggestions.

-- 
With Regards,
Amit Kapila.






^ permalink  raw  reply  [nested|flat] 3+ messages in thread

* Re: File locks for data directory lockfile in the context of Linux namespaces
@ 2026-07-10 10:20  Dmitry Dolgov <[email protected]>
  0 siblings, 0 replies; 3+ messages in thread

From: Dmitry Dolgov @ 2026-07-10 10:20 UTC (permalink / raw)
  To: Zsolt Parragi <[email protected]>; +Cc: [email protected]

> On Mon, Jul 06, 2026 at 03:07:35PM -0700, Zsolt Parragi wrote:
> +		if (errno == EAGAIN)
> +			ereport(FATAL,
> +					(errcode(ERRCODE_LOCK_FILE_EXISTS),
> 
> According to fcntl.2, this should handle both EACCESS and EAGAIN:
> 
> ERRORS
>        EACCES or EAGAIN
>               Operation is prohibited by locks held by other processes.

Good point. From what I see after a cursory look at fcntl is that it
normally returns EAGAIN, but filesystems are allowed to implement a
custom lock operation, so it makes sense to be prepared.

> +static int
> +OFDLockFile(int fd, const char *filename)
> +...
> +	else
> +		return dup(fd);
> 
> Isn't this missing an FD_CLOEXEC, so that launched processes doesn't
> inherit it and keep the lock open possibly longer than needed?

This is an interesting question. I haven't thought about this
originally, but now I think the current approach (no FD_CLOEXEC) is what
is actually needed. We want to keep the lock as long as any existing
process may access the data directory, thus the lock lifetime must be
equal to the lifetime of a longest living process. Currently the lock
file is created by the bootstrap process and the postmaster, which I
think fits the picture.

> Also, shouldn't the code verify the result of dup? (!= -1 / errno)



^ permalink  raw  reply  [nested|flat] 3+ messages in thread


end of thread, other threads:[~2026-07-10 10:20 UTC | newest]

Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-09-11 03:11 Re: Disallow altering invalidated replication slots shveta malik <[email protected]>
2024-09-12 10:54 ` Amit Kapila <[email protected]>
2026-07-10 10:20 Re: File locks for data directory lockfile in the context of Linux namespaces Dmitry Dolgov <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox