public inbox for [email protected]  

help / color / mirror / Atom feed

From: Euler Taveira <[email protected]>
To: Bharath Rupireddy <[email protected]>
To: Alvaro Herrera <[email protected]>
Cc: Amit Kapila <[email protected]>
Cc: Kyotaro Horiguchi <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: add log messages when replication slots become active and inactive (was Re: Is it worth adding ReplicationSlot active_pid to ReplicationSlotPersistentData?)
Date: Thu, 23 Mar 2023 18:40:26 -0300
Message-ID: <[email protected]> (raw)
In-Reply-To: <CALj2ACXvqFcxro64q4gjDeBNLgyPH0E6bG-WJ6w7qBqbggj3=w@mail.gmail.com>
References: <CAA4eK1+CoHVhP6hs+5-_1_A9EymSyqDPzHxBs1jSrTm5fj6CvQ@mail.gmail.com>
	<[email protected]>
	<CALj2ACXvqFcxro64q4gjDeBNLgyPH0E6bG-WJ6w7qBqbggj3=w@mail.gmail.com>

On Thu, Mar 23, 2023, at 2:33 PM, Bharath Rupireddy wrote:
> On Thu, Mar 23, 2023 at 3:37 PM Alvaro Herrera <[email protected]> wrote:
> >
> > On 2023-Mar-22, Amit Kapila wrote:
> >
> > > I see that you have modified the patch to address the comments from
> > > Alvaro. Personally, I feel it would be better to add such a message at
> > > a centralized location instead of spreading these in different callers
> > > of slot acquire/release functionality to avoid getting these missed in
> > > the new callers in the future. However, if Alvaro and others think
> > > that the current style is better then we should go ahead and do it
> > > that way. I hope that we should be able to decide on this and get it
> > > into PG16. Anyone else would like to weigh in here?
> >
> > I like Peter Smith's suggestion downthread.
> 
> +1. Please review the attached v8 patch further.
If you are adding separate functions as suggested, you should add a comment at
the top of ReplicationSlotAcquire() and ReplicationSlotRelease() functions
saying that LogReplicationSlotAquired() and LogReplicationSlotReleased()
functions should be called  respectively after it.

My suggestion is that the functions should have the same name with a "Log"
prefix. On of them has a typo "Aquired" in its name. Hence,
LogReplicationSlotAcquire() and LogReplicationSlotRelease() as names. It is
easier to find if someone is grepping by the origin function.

I prefer a sentence that includes a verb.

  physical replication slot \"%s\" is acquired
  logical replication slot \"%s\" is released

Isn't the PID important for this use case? If so, of course, you can rely on
log_line_prefix (%p) but if the PID is crucial for an investigation then it
should also be included in the message.


--
Euler Taveira
EDB   https://www.enterprisedb.com/

view thread (5+ messages)

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: add log messages when replication slots become active and inactive (was Re: Is it worth adding ReplicationSlot active_pid to ReplicationSlotPersistentData?)
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

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