Re: Transaction timeout

On Wed, Mar 13, 2024 at 7:56 AM Andrey M. Borodin wrote: > > On 13 Mar 2024, at 05:23, Alexander Korotkov wrote: > > > > On Tue, Mar 12, 2024 at 10:28 AM Andrey M. Borodin > > wrote: > >>> On 11 Mar 2024, at 16:18, Alexander Korotkov wrote: > >>> > >>> I think if checking psql stderr is proble

Re: Transaction timeout

> On 13 Mar 2024, at 05:23, Alexander Korotkov wrote: > > On Tue, Mar 12, 2024 at 10:28 AM Andrey M. Borodin > wrote: >>> On 11 Mar 2024, at 16:18, Alexander Korotkov wrote: >>> >>> I think if checking psql stderr is problematic, checking just logs is >>> fine. Could we wait for the releva

Re: Transaction timeout

On Tue, Mar 12, 2024 at 10:28 AM Andrey M. Borodin wrote: > > On 11 Mar 2024, at 16:18, Alexander Korotkov wrote: > > > > I think if checking psql stderr is problematic, checking just logs is > > fine. Could we wait for the relevant log messages one by one with > > $node->wait_for_log() just lik

Re: Transaction timeout

> On 11 Mar 2024, at 16:18, Alexander Korotkov wrote: > > I think if checking psql stderr is problematic, checking just logs is > fine. Could we wait for the relevant log messages one by one with > $node->wait_for_log() just like 040_standby_failover_slots_sync.pl do? PFA version with $node->

Re: Transaction timeout

On Mon, Mar 11, 2024 at 12:53 PM Andrey M. Borodin wrote: > > On 7 Mar 2024, at 00:55, Alexander Korotkov wrote: > > > > On Wed, Mar 6, 2024 at 10:22 AM Andrey M. Borodin > > wrote: > >>> On 25 Feb 2024, at 21:50, Alexander Korotkov wrote: > >>> > >>> Thank you for the patches. I've pushed th

Re: Transaction timeout

> On 7 Mar 2024, at 00:55, Alexander Korotkov wrote: > > On Wed, Mar 6, 2024 at 10:22 AM Andrey M. Borodin > wrote: >>> On 25 Feb 2024, at 21:50, Alexander Korotkov wrote: >>> >>> Thank you for the patches. I've pushed the 0001 patch to avoid >>> further failures on buildfarm. Let 0004 wa

Re: Transaction timeout

On Wed, Mar 6, 2024 at 10:22 AM Andrey M. Borodin wrote: > > On 25 Feb 2024, at 21:50, Alexander Korotkov wrote: > > > > Thank you for the patches. I've pushed the 0001 patch to avoid > > further failures on buildfarm. Let 0004 wait till injections points > > by Mechael are committed. > > Thank

Re: Transaction timeout

> On 25 Feb 2024, at 21:50, Alexander Korotkov wrote: > > Thank you for the patches. I've pushed the 0001 patch to avoid > further failures on buildfarm. Let 0004 wait till injections points > by Mechael are committed. Thanks! All prerequisites are committed. I propose something in a line w

Re: Transaction timeout

Hi, Andrey! On Thu, Feb 22, 2024 at 7:23 PM Andrey M. Borodin wrote: > > On 19 Feb 2024, at 15:17, Japin Li wrote: > > > > > > +1 > > PFA patch set of 4 patches: > 1. remove all potential flaky tests. BTW recently we had a bingo when 3 of > them failed together [0] > 2-3. waiting injection poin

Re: Transaction timeout

> On 19 Feb 2024, at 15:17, Japin Li wrote: > > > +1 PFA patch set of 4 patches: 1. remove all potential flaky tests. BTW recently we had a bingo when 3 of them failed together [0] 2-3. waiting injection points patchset by Michael Paquier, intact v2 from nearby thread. 4. prototype of simpl

Re: Transaction timeout

On Mon, 19 Feb 2024 at 17:14, Andrey M. Borodin wrote: >> On 18 Feb 2024, at 22:16, Andrey M. Borodin wrote: >> >> But it seems a little strange that session 3 did not fail at all > It was only coincidence. Any test that verifies FATALing out in 100ms can > fail, see new failure here [0]. > >

Re: Transaction timeout

> On 18 Feb 2024, at 22:16, Andrey M. Borodin wrote: > > But it seems a little strange that session 3 did not fail at all It was only coincidence. Any test that verifies FATALing out in 100ms can fail, see new failure here [0]. In a nearby thread Michael is proposing injections points that

Re: Transaction timeout

Alexander, thanks for pushing this! This is small but very awaited feature. > On 16 Feb 2024, at 02:08, Andres Freund wrote: > > Isn't this test going to be very fragile on busy / slow machines? What if the > pg_sleep() takes one second, because there were other tasks to schedule? I'd > be surp

Re: Transaction timeout

Hi, On 2024-02-13 23:42:35 +0200, Alexander Korotkov wrote: > diff --git a/src/backend/access/transam/xact.c > b/src/backend/access/transam/xact.c > index 464858117e0..a124ba59330 100644 > --- a/src/backend/access/transam/xact.c > +++ b/src/backend/access/transam/xact.c > @@ -2139,6 +2139,10 @@ S

Re: Transaction timeout

Hi! On Wed, Jan 31, 2024 at 11:57 AM Andrey Borodin wrote: > > On 31 Jan 2024, at 14:27, Japin Li wrote: > > > > LGTM. > > > > If there is no other objections, I'll change it to ready for committer > > next Monday. > > I think we have a quorum, so I decided to go ahead and flipped status to RfC.

Re: Transaction timeout

> On 31 Jan 2024, at 14:27, Japin Li wrote: > > LGTM. > > If there is no other objections, I'll change it to ready for committer > next Monday. I think we have a quorum, so I decided to go ahead and flipped status to RfC. Thanks! Best regards, Andrey Borodin.

Re: Transaction timeout

On Tue, 30 Jan 2024 at 14:22, Andrey M. Borodin wrote: >> On 26 Jan 2024, at 19:58, Japin Li wrote: >> >> Thanks for updating the patch. Here are some comments for v24. >> >> + >> +Terminate any session that spans longer than the specified amount of >> +time in transacti

Re: Transaction timeout

> On 26 Jan 2024, at 19:58, Japin Li wrote: > > Thanks for updating the patch. Here are some comments for v24. > > + > +Terminate any session that spans longer than the specified amount of > +time in transaction. The limit applies both to explicit transactions > +

Re: Transaction timeout

On Fri, 26 Jan 2024 at 14:44, Andrey M. Borodin wrote: >> On 22 Jan 2024, at 11:23, Peter Smith 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 necess

Re: Transaction timeout

> On 26 Jan 2024, at 11:44, Andrey M. Borodin wrote: > > > 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: ser

Re: Transaction timeout

> On 22 Jan 2024, at 11:23, Peter Smith 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! I’ve inspected CI fails and they were ca

Re: Transaction timeout

2024-01 Commitfest. 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. == [1] https://commitfest.postgresql.org/46/4040/ [2] https://cirrus-ci.com/task/4721191

Re: Transaction timeout

> On 4 Jan 2024, at 07:14, Japin Li wrote: > > Does the timeout is too short for testing? I see the timeouts for lock_timeout > and statement_timeout is more bigger than transaction_timeout. Makes sense. Done. I've also put some effort into fine-tuning timeouts Nik's case tests. To have 100ms

Re: Transaction timeout

On Wed, 03 Jan 2024 at 20:04, Andrey M. Borodin wrote: >> On 3 Jan 2024, at 16:46, Andrey M. Borodin wrote: >> >> I do not understand why, but mailing list did not pick patches that I sent. >> I'll retry. > > > Sorry for the noise. Seems like Apple updated something in Mail.App couple of > d

Re: Transaction timeout

> On 3 Jan 2024, at 16:46, Andrey M. Borodin wrote: > > I do not understand why, but mailing list did not pick patches that I sent. > I'll retry. Sorry for the noise. Seems like Apple updated something in Mail.App couple of days ago and it started to use strange "Apple-Mail" stuff by defau

Re: Transaction timeout

On 3 Jan 2024, at 11:39, Andrey M. Borodin wrote:On 1 Jan 2024, at 19:28, Andrey M. Borodin wrote: 3. Check that timeout is not rescheduled by new queries (Nik's case)The test of Nik's case was not stable enough together with COMMIT AND CHAIN. So I've s

Re: Transaction timeout

On 1 Jan 2024, at 19:28, Andrey M. Borodin wrote: 3. Check that timeout is not rescheduled by new queries (Nik's case)The test of Nik's case was not stable enough together with COMMIT AND CHAIN. So I've separated these cases into different permutations.Looking through CI logs

Re: Transaction timeout

> On 29 Dec 2023, at 16:15, Andrey M. Borodin wrote: PFA v20. Code steps are intact. Further refactored tests: 1. Check termination of active and idle queries (previously tests from Li were testing only termination of idle query) 2. Check timeout reschedule (even when last active quer

Re: Transaction timeout

> On 29 Dec 2023, at 16:00, Junwang Zhao wrote: > > After exploring the code, I found scheduling the timeout in > `StartTransaction` might be a reasonable idea, all the chain > commands will call this function. > > What concerns me is that it is also called by StartParallelWorkerTransaction,

Re: Transaction timeout

On Fri, Dec 29, 2023 at 6:00 PM Andrey M. Borodin wrote: > > > > > On 28 Dec 2023, at 21:02, Junwang Zhao wrote: > > > > Seems V5~V17 doesn't work as expected for Nikolay's case: > > > > Yeah, that's a problem. > > So I propose the following change, what do you think? > This breaks COMMIT AND CHA

Re: Transaction timeout

> On 28 Dec 2023, at 21:02, Junwang Zhao wrote: > > Seems V5~V17 doesn't work as expected for Nikolay's case: > Yeah, that's a problem. > So I propose the following change, what do you think? This breaks COMMIT AND CHAIN. PFA v18: I've added a test for Nik's case and for COMMIT AND CHAIN. No

Re: Transaction timeout

Hey Andrey, On Sun, Dec 24, 2023 at 1:14 AM Andrey M. Borodin wrote: > > > > > On 22 Dec 2023, at 10:39, Japin Li wrote: > > > > > > I try to split the test for transaction timeout, and all passed on my CI > > [1]. > > > I like the refactoring you did in timeout.spec. I thought it is impossible

Re: Transaction timeout

On Sun, 24 Dec 2023 at 01:14, Andrey M. Borodin wrote: >> On 22 Dec 2023, at 10:39, Japin Li wrote: >> >> >> I try to split the test for transaction timeout, and all passed on my CI [1]. > > > I like the refactoring you did in timeout.spec. I thought it is impossible, > because permutations wo

Re: Transaction timeout

Hi Andrey, On Sun, Dec 24, 2023 at 1:14 AM Andrey M. Borodin wrote: > > > > > On 22 Dec 2023, at 10:39, Japin Li wrote: > > > > > > I try to split the test for transaction timeout, and all passed on my CI > > [1]. > > > I like the refactoring you did in timeout.spec. I thought it is impossible,

Re: Transaction timeout

> On 22 Dec 2023, at 10:39, Japin Li wrote: > > > I try to split the test for transaction timeout, and all passed on my CI [1]. I like the refactoring you did in timeout.spec. I thought it is impossible, because permutations would try to reinitialize FATALed sessions. But, obviously, tests

Re: Transaction timeout

> 在 2023年12月23日，11:35，Junwang Zhao 写道： > > On Sat, Dec 23, 2023 at 11:17 AM Japin Li wrote: >> >> a >>> On Sat, 23 Dec 2023 at 10:40, Japin Li wrote: >>> On Sat, 23 Dec 2023 at 08:32, Japin Li wrote: On Fri, 22 Dec 2023 at 23:30, Junwang Zhao wrote: > On Fri, Dec 22, 2023 at 10:4

Re: Transaction timeout

On Sat, Dec 23, 2023 at 11:17 AM Japin Li wrote: > > a > On Sat, 23 Dec 2023 at 10:40, Japin Li wrote: > > On Sat, 23 Dec 2023 at 08:32, Japin Li wrote: > >> On Fri, 22 Dec 2023 at 23:30, Junwang Zhao wrote: > >>> On Fri, Dec 22, 2023 at 10:44 PM Japin Li wrote: > > > On Fri, 22

Re: Transaction timeout

a On Sat, 23 Dec 2023 at 10:40, Japin Li wrote: > On Sat, 23 Dec 2023 at 08:32, Japin Li wrote: >> On Fri, 22 Dec 2023 at 23:30, Junwang Zhao wrote: >>> On Fri, Dec 22, 2023 at 10:44 PM Japin Li wrote: On Fri, 22 Dec 2023 at 22:37, Junwang Zhao wrote: > On Fri, Dec 22, 2023

Re: Transaction timeout

On Sat, Dec 23, 2023 at 10:40 AM Japin Li wrote: > > > On Sat, 23 Dec 2023 at 08:32, Japin Li wrote: > > On Fri, 22 Dec 2023 at 23:30, Junwang Zhao wrote: > >> On Fri, Dec 22, 2023 at 10:44 PM Japin Li wrote: > >>> > >>> > >>> On Fri, 22 Dec 2023 at 22:37, Junwang Zhao wrote: > >>> > On Fri, D

Re: Transaction timeout

On Sat, 23 Dec 2023 at 08:32, Japin Li wrote: > On Fri, 22 Dec 2023 at 23:30, Junwang Zhao wrote: >> On Fri, Dec 22, 2023 at 10:44 PM Japin Li wrote: >>> >>> >>> On Fri, 22 Dec 2023 at 22:37, Junwang Zhao wrote: >>> > On Fri, Dec 22, 2023 at 10:25 PM Japin Li wrote: >>> >> I try to set idle_

Re: Transaction timeout

On Fri, 22 Dec 2023 at 23:30, Junwang Zhao wrote: > On Fri, Dec 22, 2023 at 10:44 PM Japin Li wrote: >> >> >> On Fri, 22 Dec 2023 at 22:37, Junwang Zhao wrote: >> > On Fri, Dec 22, 2023 at 10:25 PM Japin Li wrote: >> >> I try to set idle_in_transaction_session_timeout after begin transaction,

Re: Transaction timeout

On Fri, Dec 22, 2023 at 10:44 PM Japin Li wrote: > > > On Fri, 22 Dec 2023 at 22:37, Junwang Zhao wrote: > > On Fri, Dec 22, 2023 at 10:25 PM Japin Li wrote: > >> I try to set idle_in_transaction_session_timeout after begin transaction, > >> it changes immediately, so I think transaction_timeout

Re: Transaction timeout

On Fri, 22 Dec 2023 at 22:37, Junwang Zhao wrote: > On Fri, Dec 22, 2023 at 10:25 PM Japin Li wrote: >> I try to set idle_in_transaction_session_timeout after begin transaction, >> it changes immediately, so I think transaction_timeout should also be take >> immediately. > > Ah, right, idle_in_

Re: Transaction timeout

On Fri, Dec 22, 2023 at 10:25 PM Japin Li wrote: > > > On Fri, 22 Dec 2023 at 20:29, Junwang Zhao wrote: > > On Fri, Dec 22, 2023 at 1:39 PM Japin Li wrote: > >> > >> > >> On Tue, 19 Dec 2023 at 22:06, Japin Li wrote: > >> > On Tue, 19 Dec 2023 at 18:27, Andrey M. Borodin > >> > wrote: > >> >

Re: Transaction timeout

On Fri, 22 Dec 2023 at 20:29, Junwang Zhao wrote: > On Fri, Dec 22, 2023 at 1:39 PM Japin Li wrote: >> >> >> On Tue, 19 Dec 2023 at 22:06, Japin Li wrote: >> > On Tue, 19 Dec 2023 at 18:27, Andrey M. Borodin >> > wrote: >> >>> On 19 Dec 2023, at 13:26, Andrey M. Borodin wrote: >> >>> >> >>>

Re: Transaction timeout

On Fri, Dec 22, 2023 at 1:39 PM Japin Li wrote: > > > On Tue, 19 Dec 2023 at 22:06, Japin Li wrote: > > On Tue, 19 Dec 2023 at 18:27, Andrey M. Borodin > > wrote: > >>> On 19 Dec 2023, at 13:26, Andrey M. Borodin wrote: > >>> > >>> I don’t have Windows machine, so I hope CF bot will pick this.

Re: Transaction timeout

On Tue, 19 Dec 2023 at 22:06, Japin Li wrote: > On Tue, 19 Dec 2023 at 18:27, Andrey M. Borodin wrote: >>> On 19 Dec 2023, at 13:26, Andrey M. Borodin wrote: >>> >>> I don’t have Windows machine, so I hope CF bot will pick this. >> >> I used Github CI to produce version of tests that seems to b

Re: Transaction timeout

t; > michael.paqu...@gmail.com>; Nikolay Samokhvalov ; > pgsql-hackers ; > pgsql-hackers@lists.postgresql.org > > 主题: Re: Transaction timeout > > > > On Tue, Dec 19, 2023 at 10:51 PM Junwang Zhao wrote: > > > > > > On Tue, Dec 19, 2023 at 6:27 PM Andrey

Re: Transaction timeout

drey M. Borodin > 抄送: Japin Li ; 邱宇航 ; Fujii Masao > ; Andrey Borodin ; Andres > Freund ; Michael Paquier ; > Nikolay Samokhvalov ; pgsql-hackers > ; pgsql-hackers@lists.postgresql.org > > 主题: Re: Transaction timeout > > On Tue, Dec 19, 2023 at 10:51 PM Junwan

Re: Transaction timeout

On Tue, Dec 19, 2023 at 10:51 PM Junwang Zhao wrote: > > On Tue, Dec 19, 2023 at 6:27 PM Andrey M. Borodin > wrote: > > > > > > > > > On 19 Dec 2023, at 13:26, Andrey M. Borodin wrote: > > > > > > I don’t have Windows machine, so I hope CF bot will pick this. > > > > I used Github CI to produce

Re: Transaction timeout

On Tue, Dec 19, 2023 at 6:27 PM Andrey M. Borodin wrote: > > > > > On 19 Dec 2023, at 13:26, Andrey M. Borodin wrote: > > > > I don’t have Windows machine, so I hope CF bot will pick this. > > I used Github CI to produce version of tests that seems to be is stable on > Windows. > Sorry for the n

Re: Transaction timeout

On Tue, 19 Dec 2023 at 18:27, Andrey M. Borodin wrote: >> On 19 Dec 2023, at 13:26, Andrey M. Borodin wrote: >> >> I don’t have Windows machine, so I hope CF bot will pick this. > > I used Github CI to produce version of tests that seems to be is stable on > Windows. It still failed on Window

Re: Transaction timeout

> On 19 Dec 2023, at 13:26, Andrey M. Borodin wrote: > > I don’t have Windows machine, so I hope CF bot will pick this. I used Github CI to produce version of tests that seems to be is stable on Windows. Sorry for the noise. Best regards, Andrey Borodin. v13-0001-Introduce-transaction_tim

Re: Transaction timeout

> On 19 Dec 2023, at 06:25, Japin Li wrote: > > On Windows, there still have an error: Uhhmm, yes. Connection termination looks different on windows machine. I’ve checked how this looks in relication slot tests and removed select that was observing connection failure. I don’t have Windows mac

Re: Transaction timeout

On Mon, 18 Dec 2023 at 17:40, Andrey M. Borodin wrote: >> On 18 Dec 2023, at 14:32, Japin Li wrote: >> >> >> Thanks for updating the patch > > Sorry for the noise, but commitfest bot found one more bug in handling > statement timeout. PFA v11. > On Windows, there still have an error: diff -w

Re: Transaction timeout

> On 18 Dec 2023, at 14:32, Japin Li wrote: > > > Thanks for updating the patch Sorry for the noise, but commitfest bot found one more bug in handling statement timeout. PFA v11. Best regards, Andrey Borodin. v11-0001-Introduce-transaction_timeout.patch Description: Binary data

Re: Transaction timeout

On Mon, 18 Dec 2023 at 13:49, Andrey M. Borodin wrote: >> On 16 Dec 2023, at 05:58, Japin Li wrote: >> >> >> On Fri, 15 Dec 2023 at 17:51, Andrey M. Borodin wrote: On 8 Dec 2023, at 15:29, Japin Li wrote: Thanks for updating the patch. LGTM. >>> >>> PFA v9. Changes: >>> 1. Adde

Re: Transaction timeout

> On 16 Dec 2023, at 05:58, Japin Li wrote: > > > On Fri, 15 Dec 2023 at 17:51, Andrey M. Borodin wrote: >>> On 8 Dec 2023, at 15:29, Japin Li wrote: >>> >>> Thanks for updating the patch. LGTM. >> >> PFA v9. Changes: >> 1. Added tests for idle_in_transaction_timeout >> 2. Suppress stateme

Re: Transaction timeout

On Fri, 15 Dec 2023 at 17:51, Andrey M. Borodin wrote: >> On 8 Dec 2023, at 15:29, Japin Li wrote: >> >> Thanks for updating the patch. LGTM. > > PFA v9. Changes: > 1. Added tests for idle_in_transaction_timeout > 2. Suppress statement_timeout if it’s shorter than transaction_timeout > +

Re: Transaction timeout

> On 8 Dec 2023, at 15:29, Japin Li wrote: > > Thanks for updating the patch. LGTM. PFA v9. Changes: 1. Added tests for idle_in_transaction_timeout 2. Suppress statement_timeout if it’s shorter than transaction_timeout Consider changing status of the commitfest entry if you think it’s ready f

Re: Transaction timeout

On Fri, 08 Dec 2023 at 18:08, Andrey M. Borodin wrote: >> On 8 Dec 2023, at 12:59, Japin Li wrote: >> >> >> On Thu, 07 Dec 2023 at 20:40, Andrey M. Borodin wrote: On 7 Dec 2023, at 06:25, Japin Li wrote: If idle_in_transaction_timeout is bigger than transaction_timeout, th

Re: Transaction timeout

> On 8 Dec 2023, at 12:59, Japin Li wrote: > > > On Thu, 07 Dec 2023 at 20:40, Andrey M. Borodin wrote: >>> On 7 Dec 2023, at 06:25, Japin Li wrote: >>> >>> If idle_in_transaction_timeout is bigger than transaction_timeout, >>> the idle-in-transaction timeout don't needed, right? >> Yes, I

Re: Transaction timeout

On Thu, 07 Dec 2023 at 20:40, Andrey M. Borodin wrote: >> On 7 Dec 2023, at 06:25, Japin Li wrote: >> >> If idle_in_transaction_timeout is bigger than transaction_timeout, >> the idle-in-transaction timeout don't needed, right? > Yes, I think so. > Should we disable the idle_in_transaction_ti

Re: Transaction timeout

> On 7 Dec 2023, at 06:25, Japin Li wrote: > > If idle_in_transaction_timeout is bigger than transaction_timeout, > the idle-in-transaction timeout don't needed, right? Yes, I think so. > >> TODO: as Yuhang pointed out prepared transactions must not be killed, thus >> name "transaction_time

Re: Transaction timeout

Thanks Yuhang!On 7 Dec 2023, at 13:39, 邱宇航 wrote:I read the V6 patch and found something needs to be improved.Fixed. PFA v7.Best regards, Andrey Borodin. v7-0001-Introduce-transaction_timeout.patch Description: Binary data

Re: Transaction timeout

Hi, I read the V6 patch and found something needs to be improved. Prepared transactions should also be documented. A value of zero (the default) disables the timeout. +This timeout is not applied to prepared transactions. Only transactions +with user connections are affec

Re: Transaction timeout

On Wed, 06 Dec 2023 at 21:05, Andrey M. Borodin wrote: >> On 30 Nov 2023, at 20:06, Andrey M. Borodin wrote: >> >> >> Tomorrow I plan to fix raising of the timeout when the transaction is idle. >> Renaming transaction_timeout to something else (to avoid confusion with >> prepared xacts) also s

Re: Transaction timeout

> On 30 Nov 2023, at 20:06, Andrey M. Borodin wrote: > > > Tomorrow I plan to fix raising of the timeout when the transaction is idle. > Renaming transaction_timeout to something else (to avoid confusion with > prepared xacts) also seems correct to me. Here's a v6 version of the feature. Ch

Re: Transaction timeout

> On 20 Nov 2023, at 06:33, 邱宇航 wrote: Nikolay, Peter, Fujii, Tung, Yuhang, thank you for reviewing this. I'll address feedback soon, this patch has been for a long time on my TODO list. I've started with fixing problem of COMMIT AND CHAIN by restarting timeout counter. Tomorrow I plan to fix r

Re: Transaction timeout

I test the V4 patch and found the backend does't process SIGINT while it's in secure_read. And it seems not a good choice to report ERROR during secure_read, which will turns into FATAL "terminating connection because protocol synchronization was lost". It might be much easier to terminate the

Re: Transaction timeout

On Wed, Sep 6, 2023 at 1:16 AM Fujii Masao wrote: > With the v4 patch, I found that timeout errors no longer occur during the > idle in > transaction phase. Instead, they occur when the next statement is executed. > Is this > the intended behavior? I thought some users might want to use the tran

Re: Transaction timeout

On 2023-09-06 20:32, Andrey M. Borodin wrote: Thanks for looking into this! On 6 Sep 2023, at 13:16, Fujii Masao wrote: While testing v4 patch, I noticed it doesn't handle the COMMIT AND CHAIN case correctly. When COMMIT AND CHAIN is executed, I believe the transaction timeout counter shou

Re: Transaction timeout

Thanks for looking into this! > On 6 Sep 2023, at 13:16, Fujii Masao wrote: > > While testing v4 patch, I noticed it doesn't handle the COMMIT AND CHAIN case > correctly. > When COMMIT AND CHAIN is executed, I believe the transaction timeout counter > should reset > and start from zero with th

Re: Transaction timeout

On 2022/12/19 5:53, Andrey Borodin wrote: On Wed, Dec 7, 2022 at 1:30 PM Andrey Borodin wrote: I hope to address other feedback on the weekend. Thanks for implementing this feature! While testing v4 patch, I noticed it doesn't handle the COMMIT AND CHAIN case correctly. When COMMIT AND C

Re: Transaction timeout

On 12.01.23 20:46, Andrey Borodin wrote: On Sun, Dec 18, 2022 at 12:53:31PM -0800, Andrey Borodin wrote: I've rewritten this part to correctly report all timeouts that did happen. However there's now a tricky comma-formatting code which was tested only manually. I suspect this will make transla

Re: Transaction timeout

On Fri, Jan 13, 2023 at 10:16 AM Andrey Borodin wrote: > > – it seems we could (should) have one more successful "1s wait, 3s > sleep" iteration here, ~727ms somehow wasted in a loop, quite a lot. > > I think big chunk from these 727ms were spent between "BEGIN" and > "select now(), clock_timesta

Re: Transaction timeout

Thanks for the review Nikolay! On Fri, Jan 13, 2023 at 8:03 AM Nikolay Samokhvalov wrote: > > 1) The current test set has only 2 simple cases – I'd suggest adding one more > (that one that didn't work in v1): > > gitpod=# set transaction_timeout to '20ms'; > SET > gitpod=# begin; select pg_sleep

Re: Transaction timeout

On Thu, Jan 12, 2023 at 11:47 AM Andrey Borodin wrote: > On Thu, Jan 12, 2023 at 11:24 AM Nathan Bossart > wrote: > > > > On Sun, Dec 18, 2022 at 12:53:31PM -0800, Andrey Borodin wrote: > > > I've rewritten this part to correctly report all timeouts that did > > > happen. However there's now a t

Re: Transaction timeout

On Thu, Jan 12, 2023 at 11:24 AM Nathan Bossart wrote: > > On Sun, Dec 18, 2022 at 12:53:31PM -0800, Andrey Borodin wrote: > > I've rewritten this part to correctly report all timeouts that did > > happen. However there's now a tricky comma-formatting code which was > > tested only manually. > > I

Re: Transaction timeout

On Sun, Dec 18, 2022 at 12:53:31PM -0800, Andrey Borodin wrote: > I've rewritten this part to correctly report all timeouts that did > happen. However there's now a tricky comma-formatting code which was > tested only manually. I suspect this will make translation difficult. >> > > > + ahprin

Re: Transaction timeout

On Wed, Dec 7, 2022 at 1:30 PM Andrey Borodin wrote: > I hope to address other feedback on the weekend. Andres, here's my progress on working with your review notes. > > @@ -3277,6 +3282,7 @@ ProcessInterrupts(void) > >*/ > > lock_timeout_occurred = get_timeout_indi

Re: Transaction timeout

On Wed, Dec 7, 2022 at 10:23 AM Andres Freund wrote: > > On 2022-12-03 09:41:04 -0800, Andrey Borodin wrote: > > Fixed. Added test for this. > > The tests don't pass: https://cirrus-ci.com/build/4811553145356288 > oops, sorry. Here's the fix. I hope to address other feedback on the weekend. Thank

Re: Transaction timeout

Hi, On 2022-12-03 09:41:04 -0800, Andrey Borodin wrote: > Fixed. Added test for this. The tests don't pass: https://cirrus-ci.com/build/4811553145356288 [00:54:35.337](1.251s) not ok 1 - no parameters missing from postgresql.conf.sample [00:54:35.338](0.000s) # Failed test 'no parameters miss

Re: Transaction timeout

At Mon, 5 Dec 2022 17:10:50 -0800, Andres Freund wrote in > I'm most concerned about the overhead when the timeouts are *not* > enabled. And this adds a branch to start_xact_command() and a function > call for get_timeout_active(TRANSACTION_TIMEOUT) in that case. On its > own, that's not a whole

Re: Transaction timeout

Hi, On 2022-12-06 09:44:01 +0900, Kyotaro Horiguchi wrote: > At Mon, 5 Dec 2022 15:07:47 -0800, Andres Freund wrote in > > I'm a bit worried about adding evermore branches and function calls for > > the processing of single statements. We already spend a noticable > > percentage of the cycles for

Re: Transaction timeout

At Mon, 5 Dec 2022 15:07:47 -0800, Andres Freund wrote in > I'm a bit worried about adding evermore branches and function calls for > the processing of single statements. We already spend a noticable > percentage of the cycles for a single statement in PostgresMain(), this > adds additional overh

Re: Transaction timeout

Hi, On 2022-12-05 15:41:29 -0800, Andrey Borodin wrote: > Thanks for looking into this Andres! > > On Mon, Dec 5, 2022 at 3:07 PM Andres Freund wrote: > > > > I'm a bit worried about adding evermore branches and function calls for > > the processing of single statements. We already spend a notica

Re: Transaction timeout

Thanks for looking into this Andres! On Mon, Dec 5, 2022 at 3:07 PM Andres Freund wrote: > > I'm a bit worried about adding evermore branches and function calls for > the processing of single statements. We already spend a noticable > percentage of the cycles for a single statement in PostgresMai

Re: Transaction timeout

Hi, On 2022-12-03 09:41:04 -0800, Andrey Borodin wrote: > @@ -2720,6 +2723,7 @@ finish_xact_command(void) > > if (xact_started) > { > + > CommitTransactionCommand(); > > #ifdef MEMORY_CONTEXT_CHECKING Spurious newline added. > @@ -4460,6 +4473,10 @@ PostgresMain(c

Re: Transaction timeout

The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: not tested Documentation:not tested Tested, works as expected; documentation is not yet added

Re: Transaction timeout

On Sat, Dec 3, 2022 at 9:41 AM Andrey Borodin wrote: > Fixed. Added test for this. > Thanks! Tested (gitpod: https://gitpod.io/#https://gitlab.com/NikolayS/postgres/tree/transaction_timeout-v2 ), works as expected.

Re: Transaction timeout

On Fri, Dec 2, 2022 at 10:59 PM Nikolay Samokhvalov wrote: > > But it fails in the "worst" case I've described above – a series of small > statements: Fixed. Added test for this. Open questions: 1. Docs 2. Order of reporting if happened lock_timeout, statement_timeout, and transaction_timeout s

Re: Transaction timeout

On Fri, Dec 2, 2022 at 9:18 PM Andrey Borodin wrote: > Hello, > > We have statement_timeout, idle_in_transaction_timeout, > idle_session_timeout and many more! But we have no > transaction_timeout. I've skimmed thread [0,1] about existing timeouts > and found no contraindications to implement tra