On Mon, Aug 05, 2024 at 12:32:20AM +0500, Andrey M. Borodin wrote: > I´ve found some dead code: BufMappingPartitionLockByIndex() is unused, > and unused for a long time. See patch 1.
I don't see a reason to also get rid of BufTableHashPartition(), but otherwise this looks reasonable to me. It would probably be a good idea to first check whether there are any external callers we can find. > I´ve prototyped similar GUC for anyone willing to do such experiments. > See patch 2, 4. Probably, I´ll do some experiments too, on customer's > clusters and workloads :) Like Tomas, I'm not too wild about making this a GUC. And as Heikki pointed out upthread, a good first step would be to benchmark different NUM_BUFFER_PARTITIONS settings on modern hardware. I suspect the current setting is much lower than optimal (e.g., I doubled it and saw a TPS boost for read-only pgbench on an i5-13500T), but I don't think we fully understand the performance characteristics with different settings yet. If we find that the ideal setting depends heavily on workload/hardware, then perhaps we can consider adding a GUC, but I don't think we are there yet. >> Anyway, this value is inherently a trade off. If it wasn't, we'd set it >> to something super high from the start. But having more partitions of >> the lock table has a cost too, because some parts need to acquire all >> the partition locks (and that's O(N) where N = number of partitions). > > I´ve found none such cases, actually. Or, perhaps, I was not looking good > enough. pg_buffercache iterates over buffers and releases locks. See > patch 3 to fix comments. Yeah, I think 0003 is reasonable, too. pg_buffercache stopped acquiring all the partition locks in v10 (see commit 6e65454), which is also the commit that removed all remaining uses of BufMappingPartitionLockByIndex(). In fact, I think BufMappingPartitionLockByIndex() was introduced just for this pg_buffercache use-case (see commit ea9df81). -- nathan
