Date: Thu, 08 Sep 2022 13:40:20 +0900 (JST)
Message-Id: <20220908.134020.228552226800754222.horikyota.ntt@gmail.com>
To: bdrouvot@amazon.com
Cc: sawada.mshk@gmail.com, ashutosh.bapat.oss@gmail.com,
 pgsql-hackers@postgresql.org
Subject: Re: more descriptive message for process termination due to
 max_slot_wal_keep_size
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
In-Reply-To: <95534909-b8a0-d447-b940-b91006397962@amazon.com>
References: <03ee95c3-6496-90b7-5c8b-13fe69ee73dd@amazon.com>
	<20220907.112019.758274712731312487.horikyota.ntt@gmail.com>
	<95534909-b8a0-d447-b940-b91006397962@amazon.com>
User-Agent: Mew version 6.8 on Emacs 26.1
Mime-Version: 1.0
Content-Type: Multipart/Mixed;
 boundary="--Next_Part(Thu_Sep__8_13_40_20_2022_816)--"
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/20220908.134020.228552226800754222.horikyota.ntt%40gmail.com>
Precedence: bulk

----Next_Part(Thu_Sep__8_13_40_20_2022_816)--
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

At Wed, 7 Sep 2022 12:16:29 +0200, "Drouvot, Bertrand" <bdrouvot@amazon.com> wrote in 
> Also, rounding to zero wouldn't occur with "just" displaying
> "oldestLSN - restart_lsn" (as proposed upthread).
..
> Yeah right, but that's already the case in some part of the code, like
> for example in arrayfuncs.c:

Fair points.

> >>         ereport(LOG,
> >>             (errmsg("terminating process %d to release replication slot
> >>             \"%s\" because its restart_lsn %X/%X exceeds the limit by %.1lf
> >>             MB",
> >>                 active_pid, NameStr(slotname),
> >>                 LSN_FORMAT_ARGS(restart_lsn),
> >>                 /* round-up at sub-MB */
> >>                 ceil((double) (oldestLSN - restart_lsn) / 1024 / 102.4) /
> >>                 10),
> 
> typo "/ 102.4" ?

No, it rounds the difference up to one decimal place. So it is devided
by 10 after ceil():p

> >> LOG: terminating process 49539 to release replication slot "rep1"
> >> because its restart_lsn 0/3038000 exceeds the limit by 15.8 MB
> > If the distance were 1 byte, it is shown as "0.1 MB".
> 
> Right and I'm -1 on it, I think we should stick to the "pretty" or the
> "bytes only" approach (my preference being the bytes only one).

Okay. the points you brought up above are sufficient grounds for not
doing so.  Now they are in the following format.

>> LOG: terminating process 16034 to release replication slot "rep1"
>> because its restart_lsn 0/3158000 exceeds the limit by 15368192 bytes

Thank you for the discussion, Bertrand!

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

----Next_Part(Thu_Sep__8_13_40_20_2022_816)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="v7-0001-Make-a-message-on-process-termination-more-dscrip.patch"