On Fri, 20 Jan 2023 at 00:26, vignesh C <vignesh21@gmail.com> wrote:
> CFBot shows some compilation errors as in [1], please post an updated
> version for the same:
I've attached a rebased patch.
While reading over this again, I wondered if instead of allocating the
memory for the LOCALLOCKOWNER in TopMemoryContext, maybe we should
create a Slab context as a child of TopMemoryContext and perform the
allocations there. I feel like slab might be a better option here as
it'll use slightly less memory due to it not rounding up allocations
to the next power of 2. sizeof(LOCALLOCKOWNER) == 56, so it's not a
great deal of memory, but more than nothing. The primary reason that I
think this might be a good idea is mostly around better handling of
chunk on block fragmentation in slab.c than aset.c. If we have
transactions which create a large number of locks then we may end up
growing the TopMemoryContext and never releasing the AllocBlocks and
just having a high number of 64-byte chunks left on the freelist
that'll maybe never be used again. I'm thinking slab.c might handle
that better as it'll only keep around 10 completely empty SlabBlocks
before it'll start free'ing them. The slab allocator is quite a bit
faster now as a result of d21ded75f.
I would like to get this LockReleaseAll problem finally fixed in PG16,
but I'd feel much better about this patch if it had some review from
someone who has more in-depth knowledge of the locking code.
I've also gone and adjusted all the places that upgraded the
elog(WARNING)s of local table corruption to PANIC and put them back to
use WARNING again. While I think it might be a good idea to do that,
it seems to be adding a bit more resistance to this patch which I
don't think it really needs. Maybe we can consider that in a separate
effort.
David