Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize stroing zero FPI in WAL #327

Merged
merged 2 commits into from
Nov 24, 2023
Merged

Optimize stroing zero FPI in WAL #327

merged 2 commits into from
Nov 24, 2023

Conversation

knizhnik
Copy link

Comment on lines +661 to +662
bimg.hole_offset = 0;
cbimg.hole_length = BLCKSZ;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fairly sure this doesn't result in a PageIsNew()-page during REDO.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why?
There is the following code in RestoreBlockImage:

	/* generate page, taking into account hole if necessary */
	if (bkpb->hole_length == 0)
	{
		memcpy(page, ptr, BLCKSZ);
	}
	else
	{
		memcpy(page, ptr, bkpb->hole_offset);
		/* must zero-fill the hole */
		MemSet(page + bkpb->hole_offset, 0, bkpb->hole_length);
		memcpy(page + (bkpb->hole_offset + bkpb->hole_length),
			   ptr + bkpb->hole_offset,
			   BLCKSZ - (bkpb->hole_offset + bkpb->hole_length));
	}

It is producing zero pages with hole_offset=0, hole_length=BLCKSZ, isn't it?

Copy link
Contributor

@hlinnaka hlinnaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I'm a bit sad we had to modify Postgres code for this, but happy that we didn't need to invent a new WAL record for this.

@hlinnaka
Copy link
Contributor

Please describe the problem and how this fixes it in the commit message.

@knizhnik knizhnik merged commit e3a22b7 into REL_16_STABLE_neon Nov 24, 2023
@knizhnik knizhnik deleted the zero_fpi branch November 24, 2023 07:39
knizhnik added a commit to neondatabase/neon that referenced this pull request Nov 29, 2023
## Problem

PG16 (neondatabase/postgres#327) adds new
function to SMGR: zeroextend
It's implementation in Neon actually wal-log zero pages of extended
relation.
This zero page is wal-logged using XLOG_FPI.
As far as page is zero, the hole optimization (excluding from the image
everything between pg_upper and pd_lower) doesn't work.

## Summary of changes

In case of zero page (`PageIsNull()` returns true) assume
`hole_size=BLCKSZ`

## Checklist before requesting a review

- [ ] I have performed a self-review of my code.
- [ ] If it is a core feature, I have added thorough tests.
- [ ] Do we need to implement analytics? if so did you add the relevant
metrics to the dashboard?
- [ ] If this PR requires public announcement, mark it with
/release-notes label and add several sentences in this section.

## Checklist before merging

- [ ] Do not forget to reformat commit message to not include the above
checklist

---------

Co-authored-by: Konstantin Knizhnik <knizhnik@neon.tech>
tristan957 pushed a commit that referenced this pull request Dec 11, 2023
PG16 adds new function to SMGR: zeroextend
It's implementation in Neon actually wal-log zero pages of extended relation.
This zero page is wal-logged using XLOG_FPI.
As far as page is zero, the hole optimization (excluding from the image everything between pg_upper and pd_lower) doesn't work.

This PR allows to set hole size to BLCKSZ in case of zero page (PageIsNull() returns true).
---------

Co-authored-by: Konstantin Knizhnik <knizhnik@neon.tech>
tristan957 pushed a commit that referenced this pull request Feb 5, 2024
PG16 adds new function to SMGR: zeroextend
It's implementation in Neon actually wal-log zero pages of extended relation.
This zero page is wal-logged using XLOG_FPI.
As far as page is zero, the hole optimization (excluding from the image everything between pg_upper and pd_lower) doesn't work.

This PR allows to set hole size to BLCKSZ in case of zero page (PageIsNull() returns true).
---------

Co-authored-by: Konstantin Knizhnik <knizhnik@neon.tech>
tristan957 pushed a commit that referenced this pull request Feb 5, 2024
PG16 adds new function to SMGR: zeroextend
It's implementation in Neon actually wal-log zero pages of extended relation.
This zero page is wal-logged using XLOG_FPI.
As far as page is zero, the hole optimization (excluding from the image everything between pg_upper and pd_lower) doesn't work.

This PR allows to set hole size to BLCKSZ in case of zero page (PageIsNull() returns true).
---------

Co-authored-by: Konstantin Knizhnik <knizhnik@neon.tech>
tristan957 pushed a commit that referenced this pull request Feb 6, 2024
PG16 adds new function to SMGR: zeroextend
It's implementation in Neon actually wal-log zero pages of extended relation.
This zero page is wal-logged using XLOG_FPI.
As far as page is zero, the hole optimization (excluding from the image everything between pg_upper and pd_lower) doesn't work.

This PR allows to set hole size to BLCKSZ in case of zero page (PageIsNull() returns true).
---------

Co-authored-by: Konstantin Knizhnik <knizhnik@neon.tech>
tristan957 pushed a commit that referenced this pull request May 10, 2024
PG16 adds new function to SMGR: zeroextend
It's implementation in Neon actually wal-log zero pages of extended relation.
This zero page is wal-logged using XLOG_FPI.
As far as page is zero, the hole optimization (excluding from the image everything between pg_upper and pd_lower) doesn't work.

This PR allows to set hole size to BLCKSZ in case of zero page (PageIsNull() returns true).
---------

Co-authored-by: Konstantin Knizhnik <knizhnik@neon.tech>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants