| From: | Junwang Zhao <zhjwpku(at)gmail(dot)com> |
|---|---|
| To: | "Andrey M(dot) Borodin" <x4mmm(at)yandex-team(dot)ru> |
| Cc: | Li Japin <japinli(at)hotmail(dot)com>, 邱宇航 <iamqyh(at)gmail(dot)com>, Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>, Andrey Borodin <amborodin86(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Nikolay Samokhvalov <samokhvalov(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Transaction timeout |
| Date: | 2023-12-25 02:17:50 |
| Message-ID: | CAEG8a3K1Yix8sq6p3n49_nLCmVDjk8s1GNtiRes1kxSe5ewuFw@mail.gmail.com |
| Views: | Raw Message | Whole Thread | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Andrey,
On Sun, Dec 24, 2023 at 1:14 AM Andrey M. Borodin <x4mmm(at)yandex-team(dot)ru> wrote:
>
>
>
> > On 22 Dec 2023, at 10:39, Japin Li <japinli(at)hotmail(dot)com> 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 work the way you refactored it.
> However I don't think ignoring test failures on Windows without understanding root cause is a good idea.
> Let's get back to v13 version of tests, understand why it failed, apply your test refactorings afterwards. BTW are you sure that v14 refactorings are functional equivalent of v13 tests?
>
> To go with this plan I attach slightly modified version of v13 tests in v16 patchset. The only change is timing in "sleep_there" step. I suspect that failure was induced by more coarse timer granularity on Windows. Tests were giving only 9 milliseconds for a timeout to entirely wipe away backend from pg_stat_activity. This saves testing time, but might induce false positive test flaps. So I've raised wait times to 100ms. This seems too much, but I do not have other ideas how to ensure tests stability. Maybe 50ms would be enough, I do not know. Isolation runs ~50 seconds now. I'm tempted to say that 200ms for timeouts worth it.
>
>
> As to 2nd step "Try to enable transaction_timeout during transaction", I think this makes sense. But if we are doing so, shouldn't we also allow to enable idle_in_transaction timeout in a same manner? Currently we only allow to disable other timeouts... Also, if we are already in transaction, shouldn't we also subtract current transaction span from timeout?
idle_in_transaction_session_timeout is already the behavior Japin suggested,
it is enabled before backend sends *read for query* to client.
> I think making this functionality as another step of the patchset was a good idea.
>
> Thanks!
>
>
> Best regards, Andrey Borodin.
--
Regards
Junwang Zhao
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Japin Li | 2023-12-25 02:27:32 | Re: Transaction timeout |
| Previous Message | Alexander Korotkov | 2023-12-25 01:04:36 | Re: Assert failure on 'list_member_ptr(rel->joininfo, restrictinfo)' |