On Wed, 6 Apr 2022 at 03:40, Yura Sokolov <y.sokolov@postgrespro.ru> wrote:
> I'm looking on patch and don't get some moments.
>
> `GrantLockLocal` allocates `LOCALLOCKOWNER` and links it into
> `locallock->locallockowners`. It links it regardless `owner` could be
> NULL. But then `RemoveLocalLock` does `Assert(locallockowner->owner != NULL);`.
> Why it should not fail?
>
> `GrantLockLocal` allocates `LOCALLOCKOWNER` in `TopMemoryContext`.
> But there is single `pfree(locallockowner)` in `LockReassignOwner`.
> Looks like there should be more `pfree`. Shouldn't they?
>
> `GrantLockLocal` does `dlist_push_tail`, but isn't it better to
> do `dlist_push_head`? Resource owners usually form stack, so usually
> when owner searches for itself it is last added to list.
> Then `dlist_foreach` will find it sooner if it were added to the head.
Thanks for having a look at this. It's a bit unrealistic for me to
get a look at addressing these for v15. I've pushed this one out to
the next CF.
David
