Re: max_slot_wal_keep_size and wal_keep_segments

On 2020/07/09 1:20, Alvaro Herrera wrote:
> On 2020-Jul-09, Fujii Masao wrote:
> 
>> I attached the patch that renames wal_keep_segments to wal_keep_size.
> 
> Looks good in a quick once-over.  Just two small wording comments:

Thanks for review comments!


> 
>>     <para>
>>      Independently of <varname>max_wal_size</varname>,
>> -   <xref linkend="guc-wal-keep-segments"/> + 1 most recent WAL files are
>> +   most recent <xref linkend="guc-wal-keep-size"/> megabytes
>> +   WAL files plus one WAL file are
>>      kept at all times. Also, if WAL archiving is used, old segments can not be
>>      removed or recycled until they are archived. If WAL archiving cannot keep up
>>      with the pace that WAL is generated, or if <varname>archive_command</varname>
> 
> This reads a little strange to me.  Maybe "the N most recent megabytes
> plus ..."

Yes, fixed.


> 
>>   			/* determine how many segments slots can be kept by slots ... */
>> -			keepSegs = XLogMBVarToSegs(max_slot_wal_keep_size_mb, wal_segment_size);
>> -			/* ... and override by wal_keep_segments as needed */
>> -			keepSegs = Max(keepSegs, wal_keep_segments);
>> +			slotKeepSegs = XLogMBVarToSegs(max_slot_wal_keep_size_mb, wal_segment_size);
>> +			/* ... and override by wal_keep_size as needed */
>> +			keepSegs = XLogMBVarToSegs(wal_keep_size_mb, wal_segment_size);
> 
> Since you change the way these two variables are used, I would not say
> "override" in the above comment, nor keep the ellipses; perhaps just
> change them to "determine how many segments can be kept by slots" and
> "ditto for wal_keep_size".

Yes, fixed.

Attached is the updated version of the patch.

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION