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: Tom Lane <tgl@sss.pgh.pa.us>, 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-04-21T20:16:43Z
Lists: pgsql-hackers 
On Thu, Apr 22, 2021 at 8:07 AM Tomas Vondra
<tomas.vondra@enterprisedb.com> wrote:
> On 4/21/21 6:30 PM, Tom Lane wrote:
> > Thomas Munro <thomas.munro@gmail.com> writes:
> >> Yeah, it would have been nice to include that but it'll have to be for
> >> v15 due to lack of time to convince myself that it was correct.  I do
> >> intend to look into more concurrency of that kind for v15.  I have
> >> pushed these patches, updated to be disabled by default.
> >
> > I have a fairly bad feeling about these patches.  I've already fixed
> > one critical bug (see 9e4114822), but I am still seeing random, hard
> > to reproduce failures in WAL replay testing.  It looks like sometimes
> > the "decoded" version of a WAL record doesn't match what I see in
> > the on-disk data, which I'm having no luck tracing down.

Ugh.  Looking into this now.  Also, this week I have been researching
a possible problem with eg ALTER TABLE SET TABLESPACE in the higher
level patch, which I'll write about soon.

> > I am not sure whether the checksum failure itself is real or a variant
> > of the seeming bad-reconstruction problem, but what I'm on about right
> > at this moment is that the error handling logic for this case seems
> > quite broken.  Why is a checksum failure only worthy of a LOG message?
> > Why is ValidXLogRecord() issuing a log message for itself, rather than
> > being tied into the report_invalid_record() mechanism?  Why are we
> > evidently still trying to decode records afterwards?
>
> Yeah, that seems suspicious.

I may have invited trouble by deciding to rebase on the other proposal
late in the cycle.  That interfaces around there.

> > In general, I'm not too pleased with the apparent attitude in this
> > thread that it's okay to push a patch that only mostly works on the
> > last day of the dev cycle and plan to stabilize it later.
>
> Was there such attitude? I don't think people were arguing for pushing a
> patch's not working correctly. The discussion was mostly about getting
> it committed even and leaving some optimizations for v15.

That wasn't my plan, but I admit that the timing was non-ideal.  In
any case, I'll dig into these failures and then consider options.
More soon.

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