Skip hole in log_newpage

The log_newpage function, used to WAL-log a full copy of a page, is
missing the trick we normally use for full-page images to leave out the
unused space on the block. That's pretty trivial to implement, so we should.

The place where this matters the most is when building a new B-tree
index. When wal_level > minimal, all pages in the created index are
logged with log_newpage, and by default we leave 10% free space on index
pages. So implementing this reduces the amount of WAL generated by index
creation by roughly 10%.

Anyone see a problem with this?

- Heikki

Hi,

On 2013-12-03 13:03:41 +0200, Heikki Linnakangas wrote:
> The log_newpage function, used to WAL-log a full copy of a page, is missing
> the trick we normally use for full-page images to leave out the unused space
> on the block. That's pretty trivial to implement, so we should.
>
> The place where this matters the most is when building a new B-tree index.
> When wal_level > minimal, all pages in the created index are logged with
> log_newpage, and by default we leave 10% free space on index pages. So
> implementing this reduces the amount of WAL generated by index creation by
> roughly 10%.

Sounds like a good idea to me.

> Anyone see a problem with this?

I haven't looked thoroughly through all callsites, but shouldn't the
vacuumlazy callsite use std = true?

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On 12/03/2013 01:37 PM, Andres Freund wrote:
> I haven't looked thoroughly through all callsites, but shouldn't the
> vacuumlazy callsite use std = true?

Well, it's logging an empty page, ie. a page full of zeros. I'm not sure
if you'd consider that a "standard" page. Like the backup-block code in
xlog.c, log_newpage actually makes a full page image without the hole if
pd_lower == 0, even if you pass std = 'true', so the end result is the same.

- Heikki

On 2013-12-03 13:57:04 +0200, Heikki Linnakangas wrote:
> On 12/03/2013 01:37 PM, Andres Freund wrote:
> >I haven't looked thoroughly through all callsites, but shouldn't the
> >vacuumlazy callsite use std = true?
>
> Well, it's logging an empty page, ie. a page full of zeros. I'm not sure if
> you'd consider that a "standard" page. Like the backup-block code in xlog.c,
> log_newpage actually makes a full page image without the hole if pd_lower ==
> 0, even if you pass std = 'true', so the end result is the same.

Hm. It should have been PageInit()ed and thus have sensible
pd_lower/upper, right? Otherwise we'd have entered thePageIsNew() branch
above it.
It's obviously not critical, but it seems like a shame to write 8kb when
it's not necessary.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On 12/03/2013 02:03 PM, Andres Freund wrote:
> On 2013-12-03 13:57:04 +0200, Heikki Linnakangas wrote:
>> On 12/03/2013 01:37 PM, Andres Freund wrote:
>>> I haven't looked thoroughly through all callsites, but shouldn't the
>>> vacuumlazy callsite use std = true?
>>
>> Well, it's logging an empty page, ie. a page full of zeros. I'm not sure if
>> you'd consider that a "standard" page. Like the backup-block code in xlog.c,
>> log_newpage actually makes a full page image without the hole if pd_lower ==
>> 0, even if you pass std = 'true', so the end result is the same.
>
> Hm. It should have been PageInit()ed and thus have sensible
> pd_lower/upper, right? Otherwise we'd have entered thePageIsNew() branch
> above it.
> It's obviously not critical, but it seems like a shame to write 8kb when
> it's not necessary.

Ah, you're right. I was thinking that that is the PageIsNew() branch,
but it's not.

- Heikki

On Tue, Dec 3, 2013 at 6:03 AM, Heikki Linnakangas
<hlinnakangas(at)vmware(dot)com> wrote:
> The log_newpage function, used to WAL-log a full copy of a page, is missing
> the trick we normally use for full-page images to leave out the unused space
> on the block. That's pretty trivial to implement, so we should.
>
> The place where this matters the most is when building a new B-tree index.
> When wal_level > minimal, all pages in the created index are logged with
> log_newpage, and by default we leave 10% free space on index pages. So
> implementing this reduces the amount of WAL generated by index creation by
> roughly 10%.
>
> Anyone see a problem with this?

Looks good to me.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Heikki Linnakangas <hlinnakangas(at)vmware(dot)com> writes:
> The log_newpage function, used to WAL-log a full copy of a page, is
> missing the trick we normally use for full-page images to leave out the
> unused space on the block. That's pretty trivial to implement, so we should.

> Anyone see a problem with this?

+1. Don't forget to bump the WAL version identifier.

regards, tom lane