On Thu, Mar 12, 2020 at 5:28 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Thu, Mar 12, 2020 at 11:15 AM Dilip Kumar <dilipbal...@gmail.com> wrote: > > > > I have fixed this in the attached patch set. > > > > I have modified your > v4-0003-Conflict-Extension-Page-lock-in-group-member patch. The > modifications are (a) Change src/backend/storage/lmgr/README to > reflect new behaviour, (b) Introduce a new macro LOCK_LOCKTAG which > slightly simplifies the code, (c) moved the deadlock.c check a few > lines up and (d) changed a few comments.
Changes look fine to me. > It might be better if we can move the checks related to extension and > page lock in a separate API or macro. What do you think? I feel it looks cleaner this way as well. But, If we plan to move it to common function/macro then we should use some common name such that it can be used in FindLockCycleRecurseMember as well as in LockCheckConflicts. > I have also used an extension to test this patch. This is the same > extension that I have used to test the group locking patch. It will > allow backends to form a group as we do for parallel workers. The > extension is attached to this email. > > Test without patch: > Session-1 > Create table t1(c1 int, c2 char(500)); > Select become_lock_group_leader(); > > Insert into t1 values(generate_series(1,100),'aaa'); -- stop this > after acquiring relation extension lock via GDB. > > Session-2 > Select become_lock_group_member(); > Insert into t1 values(generate_series(101,200),'aaa'); > - Debug LockAcquire and found that it doesn't generate conflict for > Relation Extension lock. > > The above experiment has shown that without patch group members can > acquire relation extension lock if the group leader has that lock. > After patch the second session waits for the first session to release > the relation extension lock. I know this is not a perfect way to test, > but it is better than nothing. I think we need to do some more > testing either using this extension or some other way for extension > and page locks. I have also tested the same and verified it. -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com