Horton
Menu

Re: WIP: WAL prefetch (another approach)

Thomas Munro <thomas.munro@gmail.com>

View thread
From: Thomas Munro <thomas.munro@gmail.com>
To: Tomas Vondra <tomas.vondra@enterprisedb.com>
Cc: Stephen Frost <sfrost@snowman.net>, Andres Freund <andres@anarazel.de>, Jakub Wartak <Jakub.Wartak@tomtom.com>, Alvaro Herrera <alvherre@2ndquadrant.com>, Tomas Vondra <tomas.vondra@2ndquadrant.com>, Dmitry Dolgov <9erthalion6@gmail.com>, David Steele <david@pgmasters.net>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2021-03-18T00:54:18Z
Lists: pgsql-hackers

Attachments

On Thu, Mar 18, 2021 at 12:00 PM Tomas Vondra
<tomas.vondra@enterprisedb.com> wrote:
> On 3/17/21 10:43 PM, Stephen Frost wrote:
> > Guess I'm just not a fan of pushing out a change that will impact
> > everyone by default, in a possibly negative way (or positive, though
> > that doesn't seem terribly likely, but who knows), without actually
> > measuring what that impact will look like in those more common cases.
> > Showing that it's a great win when you're on ZFS or running with FPWs
> > disabled is good and the expected best case, but we should be
> > considering the worst case too when it comes to performance
> > improvements.
> >
>
> Well, maybe it'll behave differently on systems with ZFS. I don't know,
> and I have no such machine to test that at the moment. My argument
> however remains the same - if if happens to be a problem, just don't
> enable (or disable) the prefetching, and you get the current behavior.

I see the road map for this feature being to get it working on every
OS via the AIO patchset, in later work, hopefully not very far in the
future (in the most portable mode, you get I/O worker processes doing
pread() or preadv() calls on behalf of recovery).  So I'll be glad to
get this infrastructure in, even though it's maybe only useful for
some people in the first release.

> FWIW I'm not sure there was a discussion or argument about what should
> be the default setting (enabled or disabled). I'm fine with not enabling
> this by default, so that people have to enable it explicitly.
>
> In a way that'd be consistent with effective_io_concurrency being 1 by
> default, which almost disables regular prefetching.

Yeah, I'm not sure but I'd be fine with disabling it by default in the
initial release.  The current patch set has it enabled, but that's
mostly for testing, it's not an opinion on how it should ship.

I've attached a rebased patch set with a couple of small changes:

1.  I abandoned the patch that proposed
pg_atomic_unlocked_add_fetch_u{32,64}() and went for a simple function
local to xlogprefetch.c that just does pg_atomic_write_u64(counter,
pg_atomic_read_u64(counter) + 1), in response to complaints from
Andres[1].

2.  I fixed a bug in ReadRecentBuffer(), and moved it into its own
patch for separate review.

I'm now looking at Horiguchi-san and Heikki's patch[2] to remove
XLogReader's callbacks, to try to understand how these two patch sets
are related.  I don't really like the way those callbacks work, and
I'm afraid had to make them more complicated.  But I don't yet know
very much about that other patch set.  More soon.

[1] https://www.postgresql.org/message-id/20201230035736.qmyrtrpeewqbidfi%40alap3.anarazel.de
[2] https://www.postgresql.org/message-id/flat/20190418.210257.43726183.horiguchi.kyotaro@lab.ntt.co.jp

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Fix recovery_prefetch docs.

  2. Prefetch data referenced by the WAL, take II.

  3. Add circular WAL decoding buffer, take II.

  4. Fix generation of ./INSTALL for the distribution tarball

  5. Revert recovery prefetching feature.

  6. Sync guc.c and postgresql.conf.sample with the SGML docs.

  7. Add information of total data processed to replication slot stats.

  8. Doc: Review for "Optionally prefetch referenced data in recovery."

  9. Add circular WAL decoding buffer.

  10. Optionally prefetch referenced data in recovery.

  11. Remove read_page callback from XLogReader.

  12. Provide ReadRecentBuffer() to re-pin buffers by ID.

  13. Provide recovery_init_sync_method=syncfs.

  14. Mark factorial operator, and postfix operators in general, as deprecated.

  15. Rationalize GetWalRcv{Write,Flush}RecPtr().

  16. Support PrefetchBuffer() in recovery.

  17. Prevent hard failures of standbys caused by recycled WAL segments