On Tue, Dec 27, 2022 at 10:24 AM wangw.f...@fujitsu.com <wangw.f...@fujitsu.com> wrote: > > Attach the new version patch which addressed all above comments and part of > comments from [1] except one comment that are being discussed. >
1. +# Test that the deadlock is detected among leader and parallel apply workers. + +$node_subscriber->append_conf('postgresql.conf', "deadlock_timeout = 1ms"); +$node_subscriber->reload; + A. I see that the other existing tests have deadlock_timeout set as 10ms, 100ms, 100s, etc. Is there a reason to keep so low here? Shall we keep it as 10ms? B. /among leader/among the leader 2. Can we leave having tests in 022_twophase_cascade to be covered by parallel mode? The two-phase and parallel apply will be covered by 023_twophase_stream, so not sure if we get any extra coverage by 022_twophase_cascade. 3. Let's combine 0001 and 0002 as both have got reviewed independently. -- With Regards, Amit Kapila.