MIME-Version: 1.0
References: <CA+E0NR4S+NC6+QHyY_vUuQZMzLhKqczMx-jJVqtjAxF6+=JwAA@mail.gmail.com>
 <94d53a41-d1f5-4fd1-8d1e-ff36ac569848@app.fastmail.com> <4DA5A159-39FD-47DB-95E5-A4F65BC5782A@yandex-team.ru>
 <53e96b5a-cc77-4afa-8103-79c69afef1a6@uni-muenster.de> <CA+E0NR4=J-pgqEAV2weymygGmk1ePym45m2AcGyi2hpa9xRvBg@mail.gmail.com>
 <CALdSSPiq+x9pRVYvYtddTQsL8=ZMvob+vSv5mHEXbe0aoPWGig@mail.gmail.com>
 <748a7ec0-9126-4f27-a2a7-e3e449d6fdd7@uni-muenster.de> <CA+E0NR6tnLDanSNwcqZM_-ReBXitAOFsoaBAZBqXYEx0Di+fwQ@mail.gmail.com>
 <8a240667-de9e-455b-b3e2-62f34ec80154@uni-muenster.de> <CA+E0NR7=V57gVR_-CHs-0W0XL7UrKdyGfT=ZiQcuCdsDcjN94g@mail.gmail.com>
 <3f1b817f-6e7a-4e01-bfc5-50ea3724225e@uni-muenster.de> <CA+E0NR6MJfE8Hkk2Q-MgWUkk=Yy_4fWdiiaAKW_Ux_hcAAtAFQ@mail.gmail.com>
In-Reply-To: <CA+E0NR6MJfE8Hkk2Q-MgWUkk=Yy_4fWdiiaAKW_Ux_hcAAtAFQ@mail.gmail.com>
From: Kirill Reshke <reshkekirill@gmail.com>
Date: Sat, 30 Nov 2024 11:58:38 +0500
Message-ID: <CALdSSPhujM+mMH5CSFM+QYzAS5h=ga8-YDOcGN1LvOZPG4_RUg@mail.gmail.com>
Subject: Re: Truncate logs by max_log_size
To: Kirill Gavrilov <diphantxm@gmail.com>
Cc: Jim Jones <jim.jones@uni-muenster.de>, "Andrey M. Borodin" <x4mmm@yandex-team.ru>, 
	Euler Taveira <euler@eulerto.com>, pgsql-hackers@lists.postgresql.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://www.postgresql.org/message-id/CALdSSPhujM%2BmMH5CSFM%2BQYzAS5h%3Dga8-YDOcGN1LvOZPG4_RUg%40mail.gmail.com>
Precedence: bulk

On Sat, 30 Nov 2024 at 01:57, Kirill Gavrilov <diphantxm@gmail.com> wrote:
>
> On Fri, Nov 29, 2024 at 10:12=E2=80=AFPM Jim Jones <jim.jones@uni-muenste=
r.de> wrote:
>>
>>
>>
>> On 29.11.24 12:05, Kirill Gavrilov wrote:
>> >   Setting max_log_size to 0 already disables truncation.
>> >   I changed guc units to bytes, just to make it more human-friendly.
>> > Did some refactoring and fixed all warnings. Set default value in
>> > postgresql.conf.
>>
>> Nice. The patch applies cleanly and the warnings are gone.
>>
>> A few other things:
>>
>> IMHO the documentation can be a bit clearer. It says to input the values
>> in bytes, but the default value is in megabytes.
>>
>> +        If greater than zero, each query logged is truncated to this
>> many bytes.
>> +        Zero disables the setting.
>> +        Default value is 5MB.
>>
>> Perhaps something like "If this value is specified without units, it is
>> taken as bytes." (see wal_keep_size)
>
>
> Added to documentation. I hope it's clear now.
>
>>
>>
>> And a pedantic note: This hint suggests that you can use TB as unit ..
>>
>> postgres=3D# set max_log_size =3D '1foo';
>> ERROR:  invalid value for parameter "max_log_size": "1foo"
>> HINT:  Valid units for this parameter are "B", "kB", "MB", "GB", and "TB=
".
>>
>> .. but it can never be used, as it exceeds the integer range:
>>
>> postgres=3D# SET max_log_size =3D '1TB';
>> ERROR:  invalid value for parameter "max_log_size": "1TB"
>> HINT:  Value exceeds integer range.
>>
>> Thanks
>>
>> --
>> Jim
>
>
> Same thing applies to log_parameter_max_length, for example.
>
> postgres=3D# set log_parameter_max_length =3D '1foo';
> ERROR:  invalid value for parameter "log_parameter_max_length": "1foo"
> HINT:  Valid units for this parameter are "B", "kB", "MB", "GB", and "TB"=
.
> postgres=3D# set log_parameter_max_length =3D '1TB';
> ERROR:  invalid value for parameter "log_parameter_max_length": "1TB"
> HINT:  Value exceeds integer range.
>
> I think we can leave it as is.


Hi

> +for (my $attempts =3D 0; $attempts < $max_attempts; $attempts++)
> +{
> + eval {
> + $current_logfiles =3D slurp_file($node->data_dir . '/current_logfiles')=
;
> + };
> + last unless $@;
> + usleep(100_000);
> +}


`usleep` in tap tests is usually a bad pattern. Do we have a chance to
test this using `wait_for_log` or similar?


--=20
Best regards,
Kirill Reshke