From: "Andrey M. Borodin" <x4mmm@yandex-team.ru>
Message-Id: <F719524D-ACE0-474D-8E45-229EFE5AE195@yandex-team.ru>
Content-Type: multipart/mixed;
	boundary="Apple-Mail=_55394A37-A3E1-4B09-A836-2D195524135A"
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3696.120.41.1.4\))
Subject: Re: pgsql: Add TAP tests for timeouts
Date: Fri, 15 Mar 2024 17:25:24 +0500
In-Reply-To: 
 <CAPpHfdvAFor_8cftdNtXvnDAyVmv77AwtLGyEa+Gb5FVMbjZ6A@mail.gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>,
 Kyotaro Horiguchi <horikyota.ntt@gmail.com>,
 akorotkov@postgresql.org,
 pgsql-committers@lists.postgresql.org
To: Alexander Korotkov <aekorotkov@gmail.com>
References: <E1rkjDR-003kjB-St@gemulon.postgresql.org>
 <20240315.104235.1835366724413653745.horikyota.ntt@gmail.com>
 <ZfPchPC6oNN71X2J@paquier.xyz>
 <2D5B6D1F-11D2-4306-B006-4FE53D5F0E6A@yandex-team.ru>
 <ZfP8St9syoccloLg@paquier.xyz>
 <CAPpHfdsp6s6yN8XUV6Ke_1SO7iKq6CWa2g2qO7J1T3E1o+a=nA@mail.gmail.com>
 <91C963B5-63CC-4FAE-9119-E226CF84112A@yandex-team.ru>
 <CAPpHfdvAFor_8cftdNtXvnDAyVmv77AwtLGyEa+Gb5FVMbjZ6A@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/F719524D-ACE0-474D-8E45-229EFE5AE195%40yandex-team.ru>
Precedence: bulk


--Apple-Mail=_55394A37-A3E1-4B09-A836-2D195524135A
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=us-ascii


> On 15 Mar 2024, at 16:30, Alexander Korotkov <aekorotkov@gmail.com> =
wrote:
>=20
> Maybe, but do you see any negative side effects of the unconditionally
> unset of flags?

Nope, just expressed possible option.

>  If not, I would prefer to keep the code simple.

IMO that's fine. Let's incorporate wording improvement from Kyotaro to =
the same patch? PFA draft for this.

Maybe it worth to wait a little to see if some other failures will pop =
up?


Best regards, Andrey Borodin.

--Apple-Mail=_55394A37-A3E1-4B09-A836-2D195524135A
Content-Disposition: attachment;
	filename=v2-0001-Fix-race-condition-in-transaction-timeout-TAP-tes.patch
Content-Type: application/octet-stream;
	x-unix-mode=0644;
	name="v2-0001-Fix-race-condition-in-transaction-timeout-TAP-tes.patch"
Content-Transfer-Encoding: quoted-printable

=46rom=20b79d828ea4958a3ef6a1e79736c12683cfab4bb2=20Mon=20Sep=2017=20=
00:00:00=202001=0AFrom:=20"Andrey=20M.=20Borodin"=20<x4mmm@night.local>=0A=
Date:=20Fri,=2015=20Mar=202024=2017:18:19=20+0500=0ASubject:=20[PATCH=20=
v2]=20Fix=20race=20condition=20in=20transaction=20timeout=20TAP=20tests=0A=
=0AInterruption=20handler=20within=20injection=20point=20can=20stuck=20=
in=20infinite=0Aloop=20while=20handling=20transaction=20timeout.=20To=20=
avoid=20this=20situation=0Awe=20reset=20timeout=20flag=20before=20=
invoking=20injection=20point.=0A=0AAuthor:=20Alexander=20Korotkov=0A=
Reviewed-by:=20Andrey=20Borodin=0ADiscussion:=20=
https://www.postgresql.org/message-id/flat/E1rkjDR-003kjB-St%40gemulon.pos=
tgresql.org=0A---=0A=20src/backend/tcop/postgres.c=20=20=20=20=20=20=20=20=
=20=20=20=20=20=20=20=20=20=20|=2012=20+++++-------=0A=20=
src/test/modules/test_misc/t/005_timeouts.pl=20|=2012=20++++++------=0A=20=
2=20files=20changed,=2011=20insertions(+),=2013=20deletions(-)=0A=0Adiff=20=
--git=20a/src/backend/tcop/postgres.c=20b/src/backend/tcop/postgres.c=0A=
index=207ac623019b..fd4199a098=20100644=0A---=20=
a/src/backend/tcop/postgres.c=0A+++=20b/src/backend/tcop/postgres.c=0A@@=20=
-3409,8=20+3409,10=20@@=20ProcessInterrupts(void)=0A=20=09=09/*=0A=20=09=09=
=20*=20If=20the=20GUC=20has=20been=20reset=20to=20zero,=20ignore=20the=20=
signal.=20=20This=20is=0A=20=09=09=20*=20important=20because=20the=20GUC=20=
update=20itself=20won't=20disable=20any=20pending=0A-=09=09=20*=20=
interrupt.=0A+=09=09=20*=20interrupt.=20=20We=20need=20to=20unset=20the=20=
flag=20before=20the=20injection=20point,=0A+=09=09=20*=20otherwise=20we=20=
could=20loop=20in=20interrupts=20checking.=0A=20=09=09=20*/=0A+=09=09=
IdleInTransactionSessionTimeoutPending=20=3D=20false;=0A=20=09=09if=20=
(IdleInTransactionSessionTimeout=20>=200)=0A=20=09=09{=0A=20=09=09=09=
INJECTION_POINT("idle-in-transaction-session-timeout");=0A@@=20-3418,13=20=
+3420,12=20@@=20ProcessInterrupts(void)=0A=20=09=09=09=09=09=
(errcode(ERRCODE_IDLE_IN_TRANSACTION_SESSION_TIMEOUT),=0A=20=09=09=09=09=09=
=20errmsg("terminating=20connection=20due=20to=20idle-in-transaction=20=
timeout")));=0A=20=09=09}=0A-=09=09else=0A-=09=09=09=
IdleInTransactionSessionTimeoutPending=20=3D=20false;=0A=20=09}=0A=20=0A=20=
=09if=20(TransactionTimeoutPending)=0A=20=09{=0A=20=09=09/*=20As=20=
above,=20ignore=20the=20signal=20if=20the=20GUC=20has=20been=20reset=20=
to=20zero.=20*/=0A+=09=09TransactionTimeoutPending=20=3D=20false;=0A=20=09=
=09if=20(TransactionTimeout=20>=200)=0A=20=09=09{=0A=20=09=09=09=
INJECTION_POINT("transaction-timeout");=0A@@=20-3432,13=20+3433,12=20@@=20=
ProcessInterrupts(void)=0A=20=09=09=09=09=09=
(errcode(ERRCODE_TRANSACTION_TIMEOUT),=0A=20=09=09=09=09=09=20=
errmsg("terminating=20connection=20due=20to=20transaction=20timeout")));=0A=
=20=09=09}=0A-=09=09else=0A-=09=09=09TransactionTimeoutPending=20=3D=20=
false;=0A=20=09}=0A=20=0A=20=09if=20(IdleSessionTimeoutPending)=0A=20=09=
{=0A=20=09=09/*=20As=20above,=20ignore=20the=20signal=20if=20the=20GUC=20=
has=20been=20reset=20to=20zero.=20*/=0A+=09=09IdleSessionTimeoutPending=20=
=3D=20false;=0A=20=09=09if=20(IdleSessionTimeout=20>=200)=0A=20=09=09{=0A=
=20=09=09=09INJECTION_POINT("idle-session-timeout");=0A@@=20-3446,8=20=
+3446,6=20@@=20ProcessInterrupts(void)=0A=20=09=09=09=09=09=
(errcode(ERRCODE_IDLE_SESSION_TIMEOUT),=0A=20=09=09=09=09=09=20=
errmsg("terminating=20connection=20due=20to=20idle-session=20=
timeout")));=0A=20=09=09}=0A-=09=09else=0A-=09=09=09=
IdleSessionTimeoutPending=20=3D=20false;=0A=20=09}=0A=20=0A=20=09/*=0A=
diff=20--git=20a/src/test/modules/test_misc/t/005_timeouts.pl=20=
b/src/test/modules/test_misc/t/005_timeouts.pl=0Aindex=20=
e67b3e694b..dc45213685=20100644=0A---=20=
a/src/test/modules/test_misc/t/005_timeouts.pl=0A+++=20=
b/src/test/modules/test_misc/t/005_timeouts.pl=0A@@=20-44,7=20+44,7=20@@=20=
$psql_session->query_until(=0A=20=20=20=20\q=0A=20));=0A=20=0A-#=20Wait=20=
until=20the=20backend=20is=20in=20the=20timeout=20injection=20point.=20=
Will=20get=20an=20error=0A+#=20Wait=20until=20the=20backend=20enters=20=
the=20timeout=20injection=20point.=20Will=20get=20an=20error=0A=20#=20=
here=20if=20anything=20goes=20wrong.=0A=20$node->wait_for_event('client=20=
backend',=20'transaction-timeout');=0A=20=0A@@=20-58,13=20+58,13=20@@=20=
$node->safe_psql('postgres',=0A=20$node->wait_for_log('terminating=20=
connection=20due=20to=20transaction=20timeout',=0A=20=09$log_offset);=0A=20=
=0A-#=20If=20we=20send=20\q=20with=20$psql_session->quit=20it=20can=20=
get=20to=20pump=20already=20closed.=0A-#=20So=20\q=20is=20in=20initial=20=
script,=20here=20we=20only=20finish=20IPC::Run.=0A+#=20If=20we=20send=20=
\q=20with=20$psql_session->quit=20the=20command=20can=20be=20sent=20to=20=
the=20session=0A+#=20already=20closed.=20So=20\q=20is=20in=20initial=20=
script,=20here=20we=20only=20finish=20IPC::Run.=0A=20=
$psql_session->{run}->finish;=0A=20=0A=20=0A=20#=0A-#=202.=20Test=20of=20=
the=20sidle=20in=20transaction=20timeout=0A+#=202.=20Test=20of=20the=20=
idle=20in=20transaction=20timeout=0A=20#=0A=20=0A=20=
$node->safe_psql('postgres',=0A@@=20-80,7=20+80,7=20@@=20=
$psql_session->query_until(=0A=20=20=20=20BEGIN;=0A=20));=0A=20=0A-#=20=
Wait=20until=20the=20backend=20is=20in=20the=20timeout=20injection=20=
point.=0A+#=20Wait=20until=20the=20backend=20enters=20the=20timeout=20=
injection=20point.=0A=20$node->wait_for_event('client=20backend',=0A=20=09=
'idle-in-transaction-session-timeout');=0A=20=0A@@=20-111,7=20+111,7=20=
@@=20$psql_session->query_until(=0A=20=20=20=20SET=20=
idle_session_timeout=20to=20'10ms';=0A=20));=0A=20=0A-#=20Wait=20until=20=
the=20backend=20is=20in=20the=20timeout=20injection=20point.=0A+#=20Wait=20=
until=20the=20backend=20enters=20the=20timeout=20injection=20point.=0A=20=
$node->wait_for_event('client=20backend',=20'idle-session-timeout');=0A=20=
=0A=20$log_offset=20=3D=20-s=20$node->logfile;=0A--=20=0A2.37.1=20(Apple=20=
Git-137.1)=0A=0A=

--Apple-Mail=_55394A37-A3E1-4B09-A836-2D195524135A--