Re: Remove secondary checkpoint

On 30 October 2017 at 15:31, Michael Paquier <michael(dot)paquier(at)gmail(dot)com> wrote:
> On Mon, Oct 30, 2017 at 2:22 PM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
>> On 25 October 2017 at 00:17, Michael Paquier <michael(dot)paquier(at)gmail(dot)com> wrote:
>>> - * Delete old log files (those no longer needed even for previous
>>> - * checkpoint or the standbys in XLOG streaming).
>>> + * Delete old log files and recycle them
>>> */
>>> Here that's more "Delete or recycle old log files". Recycling of a WAL
>>> segment is its renaming into a newer segment.
>>
>> Sometimes files are deleted if there are too many.
>
> Sure, but one segment is never deleted and then recycled, which is
> what your new comment implies as I understand it.

I guess it depends how you read it.

The function performs both deletion AND recycling

Yet one file is only ever deleted OR recycled

>>> - checkPointLoc = ControlFile->prevCheckPoint;
>>> + /*
>>> + * It appears to be a bug that we used to use
>>> prevCheckpoint here
>>> + */
>>> + checkPointLoc = ControlFile->checkPoint;
>>> Er, no. This is correct because we expect the prior checkpoint to
>>> still be present in the event of a failure detecting the latest
>>> checkpoint.
>>
>> I'm removing "prevCheckPoint", so not sure what you mean.
>
> I mean that there is no actual bug here. And changing the code as you
> do is correct, but the comment is not.

Thanks

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services