Обсуждение: Make use of assign_checkpoint_completion_target() to calculate CheckPointSegments correctly

Hi,

It looks like assign_checkpoint_completion_target() is defined [1],
but never used, because of which CheckPointSegments may miss to
account for changed checkpoint_completion_target. I'm attaching a tiny
patch to fix this.

Thoughts?

[1]
commit 88e982302684246e8af785e78a467ac37c76dee9
Author: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date:   Mon Feb 23 18:53:02 2015 +0200

    Replace checkpoint_segments with min_wal_size and max_wal_size.

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

On Mon, Dec 26, 2022 at 06:12:34PM +0530, Bharath Rupireddy wrote:
> It looks like assign_checkpoint_completion_target() is defined [1],
> but never used, because of which CheckPointSegments may miss to
> account for changed checkpoint_completion_target. I'm attaching a tiny
> patch to fix this.
>
> Thoughts?

Oops.  It looks like you are right here.  This would impact the
calculation of CheckPointSegments on reload when
checkpoint_completion_target is updated.  This is wrong since we've
switched to max_wal_size as of 88e9823, so this had better be
backpatched all the way down.

Thoughts?
--
Michael

On Tue, Jan 17, 2023 at 12:31 PM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Mon, Dec 26, 2022 at 06:12:34PM +0530, Bharath Rupireddy wrote:
> > It looks like assign_checkpoint_completion_target() is defined [1],
> > but never used, because of which CheckPointSegments may miss to
> > account for changed checkpoint_completion_target. I'm attaching a tiny
> > patch to fix this.
> >
> > Thoughts?
>
> Oops.  It looks like you are right here.  This would impact the
> calculation of CheckPointSegments on reload when
> checkpoint_completion_target is updated.  This is wrong since we've
> switched to max_wal_size as of 88e9823, so this had better be
> backpatched all the way down.
>
> Thoughts?

+1 to backpatch as setting checkpoint_completion_target will not take
effect immediately.

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

On Tue, Jan 17, 2023 at 07:55:53PM +0530, Bharath Rupireddy wrote:
> On Tue, Jan 17, 2023 at 12:31 PM Michael Paquier <michael@paquier.xyz> wrote:
>> Oops.  It looks like you are right here.  This would impact the
>> calculation of CheckPointSegments on reload when
>> checkpoint_completion_target is updated.  This is wrong since we've
>> switched to max_wal_size as of 88e9823, so this had better be
>> backpatched all the way down.
>>
>> Thoughts?
>
> +1 to backpatch as setting checkpoint_completion_target will not take
> effect immediately.

Okay, applied down to 11.  I have double-checked the surroundings to
see if there was a similar mistake or something hiding around
CheckpointSegments but noticed nothing (also did some primary/standby
pgbench-ing with periodic reloads of checkpoint_completion_target,
while on it).
--
Michael