Re: WIP: WAL prefetch (another approach)

From: Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
To: Julien Rouhaud <rjuju123(at)gmail(dot)com>
Cc: Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com>, Daniel Gustafsson <daniel(at)yesql(dot)se>, Stephen Frost <sfrost(at)snowman(dot)net>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, David Steele <david(at)pgmasters(dot)net>, Dmitry Dolgov <9erthalion6(at)gmail(dot)com>, Jakub Wartak <Jakub(dot)Wartak(at)tomtom(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: WIP: WAL prefetch (another approach)
Date: 2022-04-07 07:45:23
Message-ID: CA+hUKGJ3aq3ZAxfhuPvMkOKwirfqunrV-p8d7kEe_WunjGndDw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Apr 4, 2022 at 3:12 PM Julien Rouhaud <rjuju123(at)gmail(dot)com> wrote:
> [review]

Thanks! I took almost all of your suggestions about renaming things,
comments, docs and moving a magic number into a macro.

Minor changes:

1. Rebased over the shmem stats changes and others that have just
landed today (woo!). The way my simple SharedStats object works and
is reset looks a little primitive next to the shiny new stats
infrastructure, but I can always adjust that in a follow-up patch if
required.

2. It was a bit annoying that the pg_stat_recovery_prefetch view
would sometimes show stale numbers when waiting for WAL to be
streamed, since that happens at arbitrary points X bytes apart in the
WAL. Now it also happens before sleeping/waiting and when recovery
ends.

3. Last year, commit a55a9847 synchronised config.sgml with guc.c's
categories. A couple of hunks in there that modified the previous
version of this work before it all got reverted. So I've re-added the
WAL_RECOVERY GUC category, to match the new section in config.sgml.

About test coverage, the most interesting lines of xlogprefetcher.c
that stand out as unreached in a gcov report are in the special
handling for the new CREATE DATABASE in file-copy mode -- but that's
probably something to raise in the thread that introduced that new
functionality without a test. I've tested that code locally; if you
define XLOGPREFETCHER_DEBUG_LEVEL you'll see that it won't touch
anything in the new database until recovery has replayed the
file-copy.

As for current CI-vs-buildfarm blind spots that recently bit me and
others, I also tested -m32 and -fsanitize=undefined,unaligned builds.

I reran one of the quick pgbench/crash/drop-caches/recover tests I had
lying around and saw a 17s -> 6s speedup with FPW off (you need much
longer tests to see speedup with them on, so this is a good way for
quick sanity checks -- see Tomas V's results for long runs with FPWs
and curved effects).

With that... I've finally pushed the 0002 patch and will be watching
the build farm.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2022-04-07 07:45:34 Re: Extensible Rmgr for Table AMs
Previous Message Thomas Munro 2022-04-07 07:44:20 pgsql: Prefetch data referenced by the WAL, take II.