On Mon, Jun 7, 2021 at 6:04 PM Dilip Kumar <dilipbal...@gmail.com> wrote: > > I have fixed all pending review comments and also added a test case which is > working fine. >
Few observations and questions on testcase: 1. +step "s1_lock_s2" { SELECT pg_advisory_lock(2); } +step "s1_lock_s3" { SELECT pg_advisory_lock(2); } +step "s1_session" { SET spec.session = 1; } +step "s1_begin" { BEGIN; } +step "s1_insert_tbl1" { INSERT INTO tbl1 VALUES(1, repeat('a', 4000)) ON CONFLICT DO NOTHING; } +step "s1_abort" { ROLLBACK; } +step "s1_unlock_s2" { SELECT pg_advisory_unlock(2); } +step "s1_unlock_s3" { SELECT pg_advisory_unlock(2); } Here, s1_lock_s3 and s1_unlock_s3 uses 2 as identifier. Don't you need to use 3 in that part of the test? 2. In the test, there seems to be an assumption that we can unlock s2 and s3 one after another, and then both will start waiting on s-1 but isn't it possible that before s2 start waiting on s1, s3 completes its insertion and then s2 will never proceed for speculative insertion? > I haven't yet checked on the back branches. Let's discuss if we think this > patch looks fine then I can apply and test on the back branches. > Sure, that makes sense. -- With Regards, Amit Kapila.