Re: [WIP] Pipelined Recovery

On Tue, Jul 21, 2026 at 11:28 AM Imran Zaheer <imran(dot)zhir(at)gmail(dot)com> wrote:
>
> Hi
>
> I am attaching a new series of patchset.
>
> What's changed?
>
> * Fixed two bugs that were causing failure for some tap tests with
> wal_pipeline=on. First bug was that WalpipelineStop() was not calling
> WakeupRecovery() that caused the producer to keep on waiting for the
> new wal to arrive. This was causing pg_combinebackup/008_promote and
> pg_basebackup/040_pg_createsubscriber to fail. Another bug in the
> startup process was using stale values for `readFile` at the end of
> recovery (FinishWalRecovery()) causing
> recovery/035_standby_logical_decoding to fail.
>
> * KnownAssignedTransactionIdsIdleMaintenance() is normally called by
> the startup process before going into waiting. So far our pipeline we
> were skipping KnownAssignedTransactionIdsIdleMaintenance() call
> because pipeline worker was not able to call this func safely as
> worker was not aware of the procarray related context (i.e. see
> ProcArrayApplyRecoveryInfo()). This also caused
> recovery/012_subtransactions to fail.
>
> + if (AmStartupProcess())
> + KnownAssignedTransactionIdsIdleMaintenance();
>
> Now this is fixed by setting a flag before the producer starts the
> wait in WaitForWALToBecomeAvailable(). The startup process detects
> this flag and opurtunistically calls
> KnownAssignedTransactionIdsIdleMaintenance() when all queue is
> consumed. This way we will be able to call this from the consumer
> (startup process) instead of the producer (decoder).
>
> * New patch 0006, this will enable the walpipeline by default. This
> way cfbot will be able to run the tests with pipelining on all
> platforms. We can later decide whether to keep this feature on or off
> by default. But for now this is gonna help cfbot find failures due to
> the pipelining.
>
> Other than that I forgot to mention about one change in my previous
> mail. As earlier suggested by Jakub and Hayato, now the flamegraphs
> attached in the benchmark zip will show reports based on per process
> (pipeline bgw and startup process). This way the profiling will make
> more sense instead of running perf on the whole cluster.
>
> Little Summary of the idea:
>
> Decoupling the decoder from the startup process and
> offloading it to a dedicated worker. Then the startup process simply
> receives the decoded records through a shmem queue. This allows the
> startup process to spend most of the CPU time in applying the wal
> records instead of decoding and reading them. We refer to this
> approach as the wal pipelining.
>
[..]

> Benchmarks: https://drive.google.com/file/d/13FATRT3kjh_y1wWETpYQh4ZXVLNaYU4A/view?usp=sharing
> Test script: https://github.com/imranzaheer612/pg-recovery-testing

Hi Imran,

I would like to help more in the longer term, but just for now, just very fast
review:

a. with original post (v1 patch) you have reported ~42% gain for
simple-update, but now it's just down to just 14% or 31%, any idea why?

b. often in "default" small s-b scenarios (e.g. in
rec-p1-tpcb-like-def-startup-proc.svg) which seem to be much more realistic,
you seem to be having huge flat
StartReadBuffer()->GetVictionBuffer()->FlushBuffer() that seems to be pricey
(visually looks like 20-40% of the whole) and it kind of shows signs double
page checksum validation there (so in "startup" we are calculating checksums
twice). As you are trying to get most of pipelining concept (most work done
outside startup/recovery), I think bgwriter is not effective enough there (or
in any PG installation out there; check bgwriter_* GUCs, hard ceiling seems to
be: 100 pages / 0.2s = 500 pages/sec =~ just 3.9 MB/s!), so it's not
aggressive enough and it should be part of this work. Some ideas: maybe for
start, try doing some longer run (dozen of minutes) and do manual inspection
of pgstat_bgwriter to see what's happennig, maybe tune it so it is much more
effective via recovery.conf add just add avg CPU% collection of bgwriter to
the benchmark run. In theory if you could get better bgwriter efficency when
evicting the pages you would be having now 3 effective "threads" working on
recovering rather than just 2, that - in theory - should make your parts of
code/patch much more effective too I think (hard to say for sure without
running experiment :) ). So, in principle I believe that when startup/recovery
is starting doing too many FlushBuffer(), bgwriter should get kicked in the
butt and unlocked, but that might (or not?) be separate idea.

c. kind of nitpicking: there's plenty of slow "vfs" operations there and
somewhat they are involving kernel-side ntfs_file_read_iter(), are you running
this on NTFS? (/media/imran/?) Maybe try on some more standard Linux fs ? (it
still should hit pagecache most of the time, I'm simply not certain of
efficency of NTFS on Linux - never saw it working there).

-J.