Re: Truncate logs by max_log_size

On Sat, 30 Nov 2024 at 01:57, Kirill Gavrilov <diphantxm(at)gmail(dot)com> wrote:
>
> On Fri, Nov 29, 2024 at 10:12 PM Jim Jones <jim(dot)jones(at)uni-muenster(dot)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=# set max_log_size = '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=# SET max_log_size = '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=# set log_parameter_max_length = '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=# set log_parameter_max_length = '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 = 0; $attempts < $max_attempts; $attempts++)
> +{
> + eval {
> + $current_logfiles = 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?

--
Best regards,
Kirill Reshke