Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1oVs75-00078t-JW for pgsql-hackers@arkaria.postgresql.org; Wed, 07 Sep 2022 10:16:56 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1oVs74-0002FL-DJ for pgsql-hackers@arkaria.postgresql.org; Wed, 07 Sep 2022 10:16:54 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1oVs73-0002FC-Iq for pgsql-hackers@lists.postgresql.org; Wed, 07 Sep 2022 10:16:54 +0000 Received: from smtp-fw-9102.amazon.com ([207.171.184.29]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1oVs71-0007VI-F8 for pgsql-hackers@postgresql.org; Wed, 07 Sep 2022 10:16:53 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amazon.com; i=@amazon.com; q=dns/txt; s=amazon201209; t=1662545812; x=1694081812; h=message-id:date:mime-version:subject:to:cc:references: from:in-reply-to; bh=9VBymUFbrpYat67el7FaBTz8n3Xft+jckBYU5qXwiHo=; b=b7YWUbR89GzEyE87zM+0Ytkh93tfc5LHq59ek/oNxGECB2CsFM/hWbvr UB6BoH8FAtI28Y0d26TulDu5ZloBJ2NkdI438xKloOBmC/p2dSnFmAi+C 2XwOfHZ7gEcywH2JuQY0Qif6uf9orRuKslX4HxiDmIEl5Cx9CqESJafjK Q=; X-IronPort-AV: E=Sophos;i="5.93,296,1654560000"; d="scan'208,217";a="256992849" Received: from pdx4-co-svc-p1-lb2-vlan2.amazon.com (HELO email-inbound-relay-pdx-2c-d9fba5dd.us-west-2.amazon.com) ([10.25.36.210]) by smtp-border-fw-9102.sea19.amazon.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 07 Sep 2022 10:16:33 +0000 Received: from EX13D03EUC003.ant.amazon.com (pdx1-ws-svc-p6-lb9-vlan2.pdx.amazon.com [10.236.137.194]) by email-inbound-relay-pdx-2c-d9fba5dd.us-west-2.amazon.com (Postfix) with ESMTPS id C4D06452B6; Wed, 7 Sep 2022 10:16:31 +0000 (UTC) Received: from [192.168.25.177] (10.43.166.36) by EX13D03EUC003.ant.amazon.com (10.43.164.192) with Microsoft SMTP Server (TLS) id 15.0.1497.38; Wed, 7 Sep 2022 10:16:30 +0000 Content-Type: multipart/alternative; boundary="------------qEnkCbES0eQXFJW3WsYZ7xac" Message-ID: <95534909-b8a0-d447-b940-b91006397962@amazon.com> Date: Wed, 7 Sep 2022 12:16:29 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.2.1 Subject: Re: more descriptive message for process termination due to max_slot_wal_keep_size Content-Language: en-US To: Kyotaro Horiguchi CC: , , References: <20220906.145336.1494945638376688936.horikyota.ntt@gmail.com> <03ee95c3-6496-90b7-5c8b-13fe69ee73dd@amazon.com> <20220907.112019.758274712731312487.horikyota.ntt@gmail.com> From: "Drouvot, Bertrand" In-Reply-To: <20220907.112019.758274712731312487.horikyota.ntt@gmail.com> X-Originating-IP: [10.43.166.36] X-ClientProxiedBy: EX13D25EUC001.ant.amazon.com (10.43.164.138) To EX13D03EUC003.ant.amazon.com (10.43.164.192) List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --------------qEnkCbES0eQXFJW3WsYZ7xac Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit Hi, On 9/7/22 4:20 AM, Kyotaro Horiguchi wrote: > (I noticed I sent a wrong version..) > > At Tue, 6 Sep 2022 10:54:35 +0200, "Drouvot, Bertrand" wrote in >> Thanks for the new patch version!. I did not realized (sorry about >> that) that we'd need to expose byte_size_pretty(). Now I wonder if we > I didn't think we need the units larger than MB, but I used > pretty_print to prevent small number from rounding to exactly zero. Yeah makes sense. Also, rounding to zero wouldn't occur with "just" displaying "oldestLSN - restart_lsn" (as proposed upthread). > On > the other hand, in typical cases it is longer than 6 digits in bytes, > which is a bit hard to read a glance. Yeah right, but that's already the case in some part of the code, like for example in arrayfuncs.c:                 ereport(ERROR,                         (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),                          errmsg("array size exceeds the maximum allowed (%d)",                                 (int) MaxAllocSize))); >> LOG: terminating process 16034 to release replication slot "rep1" because its restart_lsn 0/3158000 exceeds the limit by 15368192 bytes >> should not simply report the number of bytes (like I can see it is >> done in many places). So something like: > .. >> + (errmsg("terminating process %d to release replication slot \"%s\" >> because its restart_lsn %X/%X exceeds the limit by %lu bytes", > .. >> and then forget about exposing/using byte_size_pretty() (that would be >> more consistent with the same kind of reporting in the existing code). >> >> What do you think? > An alterntive would be rounding up to the whole MB, or a sub-MB. > >> 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" ? >> 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). Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services:https://aws.amazon.com --------------qEnkCbES0eQXFJW3WsYZ7xac Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: 8bit

Hi,

On 9/7/22 4:20 AM, Kyotaro Horiguchi wrote:
(I noticed I sent a wrong version..)

At Tue, 6 Sep 2022 10:54:35 +0200, "Drouvot, Bertrand" <bdrouvot@amazon.com> wrote in

Thanks for the new patch version!. I did not realized (sorry about
that) that we'd need to expose byte_size_pretty(). Now I wonder if we

I didn't think we need the units larger than MB, but I used
pretty_print to prevent small number from rounding to exactly zero.

Yeah makes sense.

Also, rounding to zero wouldn't occur with "just" displaying "oldestLSN - restart_lsn" (as proposed upthread).

On
the other hand, in typical cases it is longer than 6 digits in bytes,
which is a bit hard to read a glance.

Yeah right, but that's already the case in some part of the code, like for example in arrayfuncs.c:

                ereport(ERROR,
                        (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
                         errmsg("array size exceeds the maximum allowed (%d)",
                                (int) MaxAllocSize)));

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


      

        
should not simply report the number of bytes (like I can see it is
done in many places). So something like:


      

      
..


      

        
+ (errmsg("terminating process %d to release replication slot \"%s\"
because its restart_lsn %X/%X exceeds the limit by %lu bytes",


      

      
..


      

        
and then forget about exposing/using byte_size_pretty() (that would be
more consistent with the same kind of reporting in the existing code).

What do you think?


      

      
An alterntive would be rounding up to the whole MB, or a sub-MB.



      

        
       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" ?

    

    

    

    

      

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

    

    
Regards,

    

    
-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

  


--------------qEnkCbES0eQXFJW3WsYZ7xac--