> On 22 Dec 2023, at 10:39, Japin Li <japinli@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
reinitializeFATALed 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
surethat 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
only9 milliseconds for a timeout to entirely wipe away backend from pg_stat_activity. This saves testing time, but
mightinduce false positive test flaps. So I've raised wait times to 100ms. This seems too much, but I do not have other
ideashow to ensure tests stability. Maybe 50ms would be enough, I do not know. Isolation runs ~50 seconds now. I'm
temptedto 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
othertimeouts... 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.