Make use of assign_checkpoint_completion_target() to calculate CheckPointSegments correctly

Started by Bharath Rupireddyover 3 years ago4 messageshackers
Jump to latest
#1Bharath Rupireddy
bharath.rupireddyforpostgres@gmail.com

Hi,

It looks like assign_checkpoint_completion_target() is defined [1]commit 88e982302684246e8af785e78a467ac37c76dee9 Author: Heikki Linnakangas <heikki.linnakangas@iki.fi> Date: Mon Feb 23 18:53:02 2015 +0200,
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
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

Attachments:

v1-0001-Make-use-of-assign_checkpoint_completion_target-t.patchapplication/x-patch; name=v1-0001-Make-use-of-assign_checkpoint_completion_target-t.patchDownload+1-2
#2Michael Paquier
michael@paquier.xyz
In reply to: Bharath Rupireddy (#1)
Re: Make use of assign_checkpoint_completion_target() to calculate CheckPointSegments correctly

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

#3Bharath Rupireddy
bharath.rupireddyforpostgres@gmail.com
In reply to: Michael Paquier (#2)
Re: Make use of assign_checkpoint_completion_target() to calculate CheckPointSegments correctly

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

#4Michael Paquier
michael@paquier.xyz
In reply to: Bharath Rupireddy (#3)
Re: Make use of assign_checkpoint_completion_target() to calculate CheckPointSegments correctly

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