Feedback-ID: i0c21471d:Fastmail
User-Agent: Cyrus-JMAP/3.11.0-alpha0-328-gc998c829b7-fm-20240325.002-gc998c829
MIME-Version: 1.0
Message-Id: <bbbeed26-41f9-468c-ac24-ad99f429ccd7@app.fastmail.com>
In-Reply-To: <47524383-6fd1-4b13-8c4e-fb918b1d35ba@enterprisedb.com>
References: <9a9c7e65-c47b-4400-a63e-ac6169c0860e@app.fastmail.com>
 <e8564096-3c4f-4990-ac7d-ba089739b878@app.fastmail.com>
 <CANhcyEW6-dH28gLbFc5XpDTJ6JPizU+t5g-aKUWJBf5W_Zriqw@mail.gmail.com>
 <2f0b73f3-e7c0-4e95-9712-1993442a6b64@app.fastmail.com>
 <CAA4eK1JtnzqYNQ_S+mj_XsPfgk7szxegKiVQq4-8fY1wUcUnpg@mail.gmail.com>
 <34637e7f-0330-420d-8f45-1d022962d2fe@app.fastmail.com>
 <7a970912-0b77-4942-84f7-2c9ca0bc05a5@eisentraut.org>
 <d546c4bb-92d1-4e2d-898f-48234b12ed25@app.fastmail.com>
 <CANhcyEWdTTO71v+GFYUTsOOUvHus1qccg88bRmsU9KykJqjSrQ@mail.gmail.com>
 <44de4e04-db22-40ed-8ba9-46387dc8bf83@app.fastmail.com>
 <3fa9ef0f-b277-4c13-850a-8ccc04de1406@eisentraut.org>
 <CALj2ACV85cn-cTNCTP=bPehYo9d2ddvZtfGhGq==0K+65SvufQ@mail.gmail.com>
 <65b841a8-0c2f-4953-b9cd-240fce84b90b@eisentraut.org>
 <TYCPR01MB12077907C052EC329C45AE198F5362@TYCPR01MB12077.jpnprd01.prod.outlook.com>
 <c8e92bcb-69c3-477e-93d4-6f39e030cab0@app.fastmail.com>
 <47524383-6fd1-4b13-8c4e-fb918b1d35ba@enterprisedb.com>
Date: Tue, 26 Mar 2024 17:17:15 -0300
From: "Euler Taveira" <euler@eulerto.com>
To: "Tomas Vondra" <tomas.vondra@enterprisedb.com>,
 "kuroda.hayato@fujitsu.com" <kuroda.hayato@fujitsu.com>,
 "Peter Eisentraut" <peter@eisentraut.org>,
 "Bharath Rupireddy" <bharath.rupireddyforpostgres@gmail.com>
Cc: "Amit Kapila" <amit.kapila16@gmail.com>,
 "Shlok Kyal" <shlok.kyal.oss@gmail.com>,
 "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>,
 "Michael Paquier" <michael@paquier.xyz>,
 "Andres Freund" <andres@anarazel.de>,
 "Ashutosh Bapat" <ashutosh.bapat.oss@gmail.com>,
 =?UTF-8?Q?Fabr=C3=ADzio_de_Royes_Mello?= <fabriziomello@gmail.com>,
 "vignesh C" <vignesh21@gmail.com>
Subject: Re: speed up a logical replica setup
Content-Type: multipart/alternative;
 boundary=9f09d666ed9847f2ba258435270b2c8f
Archived-At: <https://www.postgresql.org/message-id/bbbeed26-41f9-468c-ac24-ad99f429ccd7%40app.fastmail.com>
Precedence: bulk

--9f09d666ed9847f2ba258435270b2c8f
Content-Type: text/plain

On Tue, Mar 26, 2024, at 4:12 PM, Tomas Vondra wrote:
> Perhaps I'm missing something, but why is NUM_CONN_ATTEMPTS even needed?
> Why isn't recovery_timeout enough to decide if wait_for_end_recovery()
> waited long enough?

It was an attempt to decoupled a connection failure (that keeps streaming the
WAL) from recovery timeout. The NUM_CONN_ATTEMPTS guarantees that if the primary
is gone during the standby recovery process, there is a way to bail out. The
recovery-timeout is 0 (infinite) by default so you have an infinite wait without
this check. The idea behind this implementation is to avoid exiting in this
critical code path. If it times out here you might have to rebuild the standby
and start again. Amit suggested [1] that we use a value as recovery-timeout but
how high is a good value? I've already saw some long recovery process using
pglogical equivalent that timeout out after hundreds of minutes. Maybe I'm too
worried about a small percentage of cases and we should use 1h as default, for
example. It would reduce the complexity since the recovery process lacks some
progress indicators (LSN is not sufficient in this case and there isn't a
function to provide the current state -- stop applying WAL, reach target, new
timeline, etc).

If we remove the pg_stat_wal_receiver check, we should avoid infinite recovery
by default otherwise we will have some reports saying the tool is hanging when
in reality the primary has gone and WAL should be streamed.

> IMHO the test should simply pass PG_TEST_DEFAULT_TIMEOUT when calling
> pg_createsubscriber, and that should do the trick.

That's a good idea. Tests are not exercising the recovery-timeout option.

> Increasing PG_TEST_DEFAULT_TIMEOUT is what buildfarm animals doing
> things like ubsan/valgrind already use to deal with exactly this kind of
> timeout problem.
> 
> Or is there a deeper problem with deciding if the system is in recovery?

As I said with some recovery progress indicators it would be easier to make some
decisions like wait a few seconds because the WAL has already been applied and
it is creating a new timeline. The recovery timeout decision is a shot in the
dark because we might be aborting pg_createsubscriber when the target server is
about to set RECOVERY_STATE_DONE.


[1] https://www.postgresql.org/message-id/CAA4eK1JRgnhv_ySzuFjN7UaX9qxz5Hqcwew7Fk%3D%2BAbJbu0Kd9w%40mail.gmail.com


--
Euler Taveira
EDB   https://www.enterprisedb.com/

--9f09d666ed9847f2ba258435270b2c8f
Content-Type: text/html
Content-Transfer-Encoding: quoted-printable

<!DOCTYPE html><html><head><title></title><style type=3D"text/css">p.Mso=
Normal,p.MsoNoSpacing{margin:0}</style></head><body><div>On Tue, Mar 26,=
 2024, at 4:12 PM, Tomas Vondra wrote:<br></div><blockquote type=3D"cite=
" id=3D"qt" style=3D""><div>Perhaps I'm missing something, but why is NU=
M_CONN_ATTEMPTS even needed?<br></div><div>Why isn't recovery_timeout en=
ough to decide if wait_for_end_recovery()<br></div><div>waited long enou=
gh?<br></div></blockquote><div><br></div><div>It was an attempt to decou=
pled a connection failure (that keeps streaming the<br></div><div>WAL) f=
rom recovery timeout. The NUM_CONN_ATTEMPTS guarantees that if the prima=
ry<br></div><div>is gone during the standby recovery process, there is a=
 way to bail out. The<br></div><div>recovery-timeout is 0 (infinite) by =
default so you have an infinite wait without<br></div><div>this check. T=
he idea behind this implementation is to avoid exiting in this<br></div>=
<div>critical code path. If it times out here you might have to rebuild =
the standby<br></div><div>and start again. Amit suggested [1] that we us=
e a value as recovery-timeout but<br></div><div>how high is a good value=
? I've already saw some long recovery process using<br></div><div>pglogi=
cal equivalent that timeout out after hundreds of minutes. Maybe I'm too=
<br></div><div>worried about a small percentage of cases and we should u=
se 1h as default, for<br></div><div>example. It would reduce the complex=
ity since the recovery process lacks some<br></div><div>progress indicat=
ors (LSN is not sufficient in this case and there isn't a<br></div><div>=
function to provide the current state -- stop applying WAL, reach target=
, new<br></div><div>timeline, etc).<br></div><div><br></div><div>If we r=
emove the pg_stat_wal_receiver check, we should avoid infinite recovery<=
br></div><div>by default otherwise we will have some reports saying the =
tool is hanging when<br></div><div>in reality the primary has gone and W=
AL should be streamed.<br></div><div><br></div><blockquote type=3D"cite"=
 id=3D"qt" style=3D""><div>IMHO the test should simply pass PG_TEST_DEFA=
ULT_TIMEOUT when calling<br></div><div>pg_createsubscriber, and that sho=
uld do the trick.<br></div></blockquote><div><br></div><div>That's a goo=
d idea. Tests are not exercising the recovery-timeout option.</div><div>=
<br></div><blockquote type=3D"cite" id=3D"qt" style=3D""><div>Increasing=
 PG_TEST_DEFAULT_TIMEOUT is what buildfarm animals doing<br></div><div>t=
hings like ubsan/valgrind already use to deal with exactly this kind of<=
br></div><div>timeout problem.<br></div><div><br></div><div>Or is there =
a deeper problem with deciding if the system is in recovery?<br></div></=
blockquote><div><br></div><div>As I said with some recovery progress ind=
icators it would be easier to make some<br></div><div>decisions like wai=
t a few seconds because the WAL has already been applied and<br></div><d=
iv>it is creating a new timeline. The recovery timeout decision is a sho=
t in the<br></div><div>dark because we might be aborting pg_createsubscr=
iber when the target server is<br></div><div>about to set RECOVERY_STATE=
_DONE.<br></div><div><br></div><div><br></div><div><div>[1] <a href=3D"h=
ttps://www.postgresql.org/message-id/CAA4eK1JRgnhv_ySzuFjN7UaX9qxz5Hqcwe=
w7Fk%3D%2BAbJbu0Kd9w%40mail.gmail.com">https://www.postgresql.org/messag=
e-id/CAA4eK1JRgnhv_ySzuFjN7UaX9qxz5Hqcwew7Fk%3D%2BAbJbu0Kd9w%40mail.gmai=
l.com</a><br></div><div><br></div><div><br></div></div><div id=3D"sig112=
409080"><div class=3D"signature">--<br></div><div class=3D"signature">Eu=
ler Taveira<br></div><div class=3D"signature">EDB&nbsp;&nbsp; <a href=3D=
"https://www.enterprisedb.com/">https://www.enterprisedb.com/</a><br></d=
iv></div><div><br></div></body></html>
--9f09d666ed9847f2ba258435270b2c8f--