> 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. 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? I think making this functionality as another step of the patchset was a good idea. Thanks! Best regards, Andrey Borodin.
v17-0001-Introduce-transaction_timeout.patch
Description: Binary data
v17-0002-Try-to-enable-transaction_timeout-before-next-co.patch
Description: Binary data
