MIME-Version: 1.0
References: 
 <CA+UBfa=vDV8wbmAV0pgrx-FuJh+x8YOW23vJ90Jzr=14rV+9jA@mail.gmail.com>
 <OS9PR01MB12149A4E7927072A215AEED69F565A@OS9PR01MB12149.jpnprd01.prod.outlook.com>
 <CA+UBfakmkdtauuRsOVXFqhFVJt0nTdEadx94tJn+qG0Pe8Wjfw@mail.gmail.com>
 <CAN4CZFM7FV0VTNkujD=Mb7tNa+jkmEfnX7carvj95fY6Tp11FQ@mail.gmail.com>
 <CA+UBfamW6NuuMMQTDRPDQ0a9fWN_u2OvjEF98u3CfYKTBcOZMw@mail.gmail.com>
 <CA+UBfa=Dv-2tLSEKHJ0YFFH8PCTHxnX9rtVZeV8gd8q1a-GmYA@mail.gmail.com>
 <CA+UBfa=PKdShpSTTTSHwXdGPZnm2rGMKPjERNOdS0SG9t9CT3Q@mail.gmail.com>
In-Reply-To: 
 <CA+UBfa=PKdShpSTTTSHwXdGPZnm2rGMKPjERNOdS0SG9t9CT3Q@mail.gmail.com>
From: Xuneng Zhou <xunengzhou@gmail.com>
Date: Thu, 19 Mar 2026 10:33:44 +0800
Message-ID: 
 <CABPTF7WVW2x4XitXttHwCamSZcBn=Q+wLjf+M+MuEbZSAxqdDw@mail.gmail.com>
Subject: Re: [WIP] Pipelined Recovery
To: Imran Zaheer <imran.zhir@gmail.com>
Cc: Zsolt Parragi <zsolt.parragi@percona.com>,
 Jakub Wartak <jakub.wartak@enterprisedb.com>,
	"Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>,
 pgsql-hackers <pgsql-hackers@postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CABPTF7WVW2x4XitXttHwCamSZcBn%3DQ%2BwLjf%2BM%2BMuEbZSAxqdDw%40mail.gmail.com>
Precedence: bulk

Hi Imran,

On Wed, Mar 18, 2026 at 8:06=E2=80=AFPM Imran Zaheer <imran.zhir@gmail.com>=
 wrote:
>
> (resending the mail, previous mail was held for moderation for some reaso=
n.)
>
> Hi
>
> I am attaching a new rebased version of the patch. Following are some maj=
or changes in the new patch set.
>
> * Streaming replication is now working. The prefetcher was not fully deco=
upled from the startup process; that's why there were inconsistencies in so=
me scenarios and most of the recovery tap tests were failing.
>
> * Patch is now split into consumer and producer patches. This will make r=
eview easier.
>
> * Pipeline shutdown flow is also improved. Now producer will always check=
 for the shutdown flag (being set by the consumer)
>
> * Pipeline msg queue size is now configurable `wal_pipeline_queue_size`
>
> * Tap tests now passes with PG_TEST_INITDB_EXTRA_OPTS=3D"-c wal_pipeline=
=3Don"
>
> * New tap test for `recovery/t/053_walpipeline.pl`. This covers some basi=
c functionality of the pipeline.
>
> * The filename is changed to xlogpipeline.{h|C}
>
> Thanks to all for the valuable feedback.
> Looking forward to your reviews, comments, etc.
>
> --
> Regards,
> Imran Zaheer
>
>
> On Wed, Mar 18, 2026 at 3:15=E2=80=AFPM Imran Zaheer <imran.zhir@gmail.co=
m> wrote:
>>
>> (resending the mail, previous mail was held for moderation for some reas=
on. Now pdf is moved to the tar.gz)
>>
>> Hi
>>
>> I am attaching a new rebased version of the patch. Following are some ma=
jor changes in the new patch set.
>>
>> * Streaming replication is now working. The prefetcher was not fully dec=
oupled from the startup process; that's why there were inconsistencies in s=
ome scenarios and most of the recovery tap tests were failing.
>>
>> * Patch is now split into consumer and producer patches. This will make =
review easier.
>>
>> * Pipeline shutdown flow is also improved. Now producer will always chec=
k for the shutdown flag (being set by the consumer)
>>
>> * Pipeline msg queue size is now configurable `wal_pipeline_queue_size`
>>
>> * Tap tests now passes with PG_TEST_INITDB_EXTRA_OPTS=3D"-c wal_pipeline=
=3Don"
>>
>> * New tap test for `recovery/t/053_walpipeline.pl`. This covers some bas=
ic functionality of the pipeline.
>>
>> * The filename is changed to xlogpipeline.{h|C}
>>
>> Thanks to all for the valuable feedback.
>> Looking forward to your reviews, comments, etc.
>>
>> --
>> Regards,
>> Imran Zaheer
>>
>>
>> On Wed, Mar 18, 2026 at 12:43=E2=80=AFPM Imran Zaheer <imran.zhir@gmail.=
com> wrote:
>>>
>>> Hi Zsolt.
>>>
>>> Thanks alot for the review and pointing out the bugs. I have fixed the =
bugs you mentioned in my new patch set. But
>>> patchset mail is held for moderation for some reason.
>>>
>>> >
>>> >   if (reachedRecoveryTarget)
>>> >   {
>>> > + if (wal_pipeline_enabled)
>>> > + WalPipeline_Stop();
>>> >
>>> > What if we didn't reach the recovery target, shouldn't we stop the
>>> > pipelines then?
>>> >
>>>
>>> I have fixed the bugs shutdown logic.
>>>
>>> As we already know we will exist the recovery redo loop in `PerformWalR=
ecovery()` only in two cases
>>>
>>> 1: Recovery target reached:
>>> In this case consumers will call to stop the pipeline.
>>>
>>> @@ -1807,6 +1931,9 @@ PerformWalRecovery(void)
>>>
>>>   if (reachedRecoveryTarget)
>>>   {
>>> + if (wal_pipeline_enabled)
>>> + WalPipeline_Stop();
>>> +
>>>
>>> 2: Available pg_wal is consumed and now more wal to read.
>>> In this case pipeline producers will send the shutdown msg to the consu=
mer. Consumer will
>>> detect this message and then call ` WalPipeline_Stop`. This is the case=
 where we cannot read
>>> more records and the while loop will break here.
>>>
>>> + if (decoded_record)
>>> + {
>>> + record =3D &decoded_record->header;
>>> + return record;
>>> + }
>>> + else
>>> + {
>>> + /*
>>> + * We will end up here only when pipeline couldn't read more
>>> + * records and have sent a shutdown msg. We will acknowldge this
>>> + * and will trigger request to stop the pipeline workers.
>>> + */
>>> + WalPipeline_Stop();
>>> + return NULL;
>>> + }
>>>
>>>
>>> Hope this makes sense.
>>>
>>> Once again thanks for reporting the bugs. You will receive the new patc=
hset mail soon once it is cleared from
>>> the moderation.
>>>
>>> Looking forward to your reviews, comments, etc.
>>>
>>> Regards,
>>> Imran Zaheer

Thanks for this patch=E2=80=94it=E2=80=99s quite interesting. To my knowled=
ge, there
have been prior attempts to introduce parallelism into recovery, as
you mentioned in your earlier email.

I=E2=80=99m curious how this approach differs from those previous efforts, =
and
why those attempts ultimately did not land. I imagine there were
constraints or complexities involved. It would be valuable to
understand what lessons can be drawn from them.
It also raises an implicit question: what makes the current approach
more promising=E2=80=94whether due to a simpler design or improved
performance.

While these may not be directly related to your current proposal, the
insights and experience from earlier work could help guide the
development and shape the direction of this patch. Of course, some of
this context can be pieced together from mailing list discussions and
past talks, but doing so raises the bar for future reviewers. Any
additional background you can share would be very helpful.

--=20
Best,
Xuneng