public inbox for [email protected]
help / color / mirror / Atom feedFrom: Japin Li <[email protected]>
To: Andrey M. Borodin <[email protected]>
Cc: Peter Smith <[email protected]>
Cc: Junwang Zhao <[email protected]>
Cc: 邱宇航 <[email protected]>
Cc: Fujii Masao <[email protected]>
Cc: Andrey Borodin <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: Nikolay Samokhvalov <[email protected]>
Cc: pgsql-hackers <[email protected]>
Cc: [email protected] <[email protected]>
Subject: Re: Transaction timeout
Date: Fri, 26 Jan 2024 22:58:34 +0800
Message-ID: <ME3P282MB316697D91841CF4EE86CF882B6792@ME3P282MB3166.AUSP282.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <[email protected]>
References: <CAAhFRxiQsRs2Eq5kCo9nXE3HTugsAAJdSQSmxncivebAxdmBjQ@mail.gmail.com>
<ME3P282MB31665F69A72FE8427EE8C49FB694A@ME3P282MB3166.AUSP282.PROD.OUTLOOK.COM>
<CAEG8a3K8kD0mQDFzj=v0zV7h+m4J4TDBLkXSJJS3+VGWNvnDCQ@mail.gmail.com>
<ME3P282MB3166AB8DF0381F27C077AF70B69BA@ME3P282MB3166.AUSP282.PROD.OUTLOOK.COM>
<ME3P282MB31663557EEF80E7CA97FBC32B69BA@ME3P282MB3166.AUSP282.PROD.OUTLOOK.COM>
<ME3P282MB316691F6AD09F4F8EA5578C1B69BA@ME3P282MB3166.AUSP282.PROD.OUTLOOK.COM>
<CAEG8a3+nXPnceKAyFT12XBwDvEe3POvre9Z=h6j2cCiK+zL7vg@mail.gmail.com>
<ME3P282MB31664A5018CEC4782D2DA7DAB69BA@ME3P282MB3166.AUSP282.PROD.OUTLOOK.COM>
<[email protected]>
<CAEG8a3KDAWrCY5coVdfO6PNFbNJNgLOusNKE7advB8fy7_6jMw@mail.gmail.com>
<[email protected]>
<CAEG8a3J6mSPXgHq_aqC7yk4xq1e9XF9B1Q+QVBr48fzdw1uKqw@mail.gmail.com>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<ME3P282MB3166A9BFA45239291EBC5F1CB667A@ME3P282MB3166.AUSP282.PROD.OUTLOOK.COM>
<[email protected]>
<CAHut+PuOdzypMrK3HsSr0F2fyn8VO5z2+EZ=6Sz39QNbBrisHQ@mail.gmail.com>
<[email protected]>
On Fri, 26 Jan 2024 at 14:44, Andrey M. Borodin <[email protected]> wrote:
>> On 22 Jan 2024, at 11:23, Peter Smith <[email protected]> wrote:
>>
>> Hi, This patch has a CF status of "Needs Review" [1], but it seems
>> there was a CFbot test failure last time it was run [2]. Please have a
>> look and post an updated version if necessary.
> Thanks Peter!
>
Thanks for updating the patch. Here are some comments for v24.
+ <para>
+ Terminate any session that spans longer than the specified amount of
+ time in transaction. The limit applies both to explicit transactions
+ (started with <command>BEGIN</command>) and to implicitly started
+ transaction corresponding to single statement. But this limit is not
+ applied to prepared transactions.
+ If this value is specified without units, it is taken as milliseconds.
+ A value of zero (the default) disables the timeout.
+ </para>
The sentence "But this limit is not applied to prepared transactions" is redundant,
since we have a paragraph to describe this later.
+
+ <para>
+ If <varname>transaction_timeout</varname> is shorter than
+ <varname>idle_in_transaction_session_timeout</varname> or <varname>statement_timeout</varname>
+ <varname>transaction_timeout</varname> will invalidate longer timeout.
+ </para>
+
Since we are already try to disable the timeouts, should we try to disable
them even if they are equal.
+
+ <para>
+ Prepared transactions are not subject for this timeout.
+ </para>
Maybe wrap this with <note> is a good idea.
> I’ve inspected CI fails and they were caused by two different problems:
> 1. It’s unsafe for isaoltion tester to await transaction_timeout within a query. Usually it gets
> FATAL: terminating connection due to transaction timeout
> But if VM is a bit slow it can get occasional
> PQconsumeInput failed: server closed the connection unexpectedly
> So, currently all tests use “passive waiting”, in a session that will not timeout.
>
> 2. In some cases pg_sleep(0.1) were sleeping up to 200 ms. That was making s7 and s8 fail, because they rely on this margin.
I'm curious why this happened.
> I’ve separated these tests into different test timeouts-long and increased margin to 300ms. Now tests run horrible 2431 ms. Moreover I’m afraid that on buildfarm we can have much randomly-slower machines so this test might be excluded.
> This test checks COMMIT AND CHAIN and flow of small queries (Nik’s case).
>
> Also I’ve verified that every "enable_timeout_after(TRANSACTION_TIMEOUT)” and “disable_timeout(TRANSACTION_TIMEOUT)” is necessary and found that case of aborting "idle in transaction (aborted)” is not covered by tests. I’m not sure we need a test for this.
I see there is a test about idle_in_transaction_timeout and transaction_timeout.
Both of them only check the session, but don't check the reason, so we cannot
distinguish the reason they are terminated. Right?
> Japin, Junwang, what do you think?
However, checking the reason on the timeout session may cause regression test
failed (as you point in 1), I don't strongly insist on it.
--
Best regards,
Japin Li.
view thread (76+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Transaction timeout
In-Reply-To: <ME3P282MB316697D91841CF4EE86CF882B6792@ME3P282MB3166.AUSP282.PROD.OUTLOOK.COM>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox