On Sun, 24 Dec 2023 at 01:14, Andrey M. Borodin <x4...@yandex-team.ru> wrote: >> On 22 Dec 2023, at 10:39, Japin Li <japi...@hotmail.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.
Yeah. > 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? > I think it is equivalent. Maybe I missing something. Please let me known if they are not equivalent. > 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. > So this is caused by Windows timer granularity? > 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? I think the current idle_in_transaction_session_timeout work correctly. > 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? Agreed. > I think making this functionality as another step of the patchset was a good > idea. > -- Regrads, Japin Li ChengDu WenWu Information Technology Co., Ltd.
