| From: | Melanie Plageman <melanieplageman(at)gmail(dot)com> |
|---|---|
| To: | Robert Haas <robertmhaas(at)gmail(dot)com> |
| Cc: | Michael Paquier <michael(at)paquier(dot)xyz>, Rogers Wang <rogers(dot)ww(at)qq(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org, Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com>, Álvaro Herrera <alvherre(at)kurilemu(dot)de>, Andres Freund <andres(at)anarazel(dot)de> |
| Subject: | Re: WAL_LOG CREATE DATABASE strategy broken for non-standard page layouts |
| Date: | 2026-11-10 22:10:25 |
| Message-ID: | CAAKRu_b9r-Pz9gzYvC+5EHnXqNgfsJhcH=huUGbXzvcnbHtxCg@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Fri, Nov 6, 2026 at 10:13 AM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> On Mon, Oct 12, 2026 at 4:07 PM Melanie Plageman
> <melanieplageman(at)gmail(dot)com> wrote:
> > Upon further off-list discussion, instead of checking the AM, we can
> > just check if the space between pd_upper and pd_lower is all zeroes.
> > If it is, there is no harm in omitting it. It should always work,
> > allows the optimization in more cases, and requires less code.
> > Attached patch does that.
>
> The problem with this approach is that it also includes the hole data
> when the page is standard but the space between pd_upper and pd_lower
> contains leftover garbage rather than zero bytes. That case is common:
> when we prune-and-defragment heap pages, we don't zero the hole. I
> think it would be worth trying to pass down the AM OID and always use
> page_std = true for main-fork heap and btree pages (and maybe other
> in-core AMs with no known legacy problems). That would mitigate the
> practical downsides of the patch considerably. Whether you do that or
> not, it's probably worth mentioning the performance concern in the
> commit message. With that change, the downside would only exist for
> out-of-core AMs, which do exist, and they could consider vigorous
> hole-zeroing as a workaround.
I was aware that prune/defrag doesn't zero the data and so that would
not be included. I was mostly concerned that singling out heap tables
and btree felt a little inexact. But I suppose since it's a pretty
trivial diff to thread that information up from
ScanSourceDatabasePgClassTuple() to RelationCopyStorageUsingBuffer(),
it's worth doing.
Attached v2 uses a hybrid approach that passes page_std for heap
tables and btree indexes as well as when the data between pd_upper and
pd_lower is all zeroes. That allows other AMs to zero out that area if
they want to preserve the old behavior and avoid a regression (which I
think was what you were suggesting).
I also updated the test to use pg_visibilitymap_summary() instead of
comparing the raw page, which should fix test stability concerns.
The second commit in the patch set is targeting master only and would
extend this approach to RelationCopyStorage(), which would reduce the
WAL volume produced and works even more naturally there because they
already know the relation AM and page layout.
- Melanie
| Attachment | Content-Type | Size |
|---|---|---|
| v2-0001-Fix-WAL-logged-CREATE-DATABASE-with-nonstandard-p.patch | text/x-patch | 13.6 KB |
| v2-0002-Reduce-WAL-generated-when-copying-relation-storag.patch | text/x-patch | 5.6 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Thomas Munro | 2026-11-10 22:14:36 | Re: pg_threads.h take II |
| Previous Message | Ishjaap Singh | 2026-11-10 21:15:52 | Fix jsonpath printing of operations followed by an accessor |