Re: Patch: fix lock contention for HASHHDR.mutex

Andres, Robert, are you still reviewing this patch?

Aleksander Alekseev wrote:
> Here is a clean version of the patch. Step 1 is an optimization. Step 2
> refactors this:
>
> HTAB *
> ShmemInitHash(const char *name, /* table string name for shmem index */
> - long init_size, /* initial table size */
> + long init_size, /* initial table size XXX ignored,
> refactor! */

So you were saying some posts upthread that the new hash tables would
"borrow" AN elements from the freelist then put AN elements back when
too many got unused. Is that idea embodied in this patch? I'm nervous
about continuing a line of development that does away with the
"init_size" concept completely, but if this patch is keep the "borrow"
concept then maybe it doesn't matter.

> -/* Number of partitions of the shared buffer mapping hashtable */
> -#define NUM_BUFFER_PARTITIONS 128
> -
> /* Number of partitions the shared lock tables are divided into */
> -#define LOG2_NUM_LOCK_PARTITIONS 4
> +#define LOG2_NUM_LOCK_PARTITIONS 7
> #define NUM_LOCK_PARTITIONS (1 << LOG2_NUM_LOCK_PARTITIONS)
>
> +/* Number of partitions of the shared buffer mapping hashtable */
> +#define NUM_BUFFER_PARTITIONS NUM_LOCK_PARTITIONS
> +
> /* Number of partitions the shared predicate lock tables are divided into */
> #define LOG2_NUM_PREDICATELOCK_PARTITIONS 4
> #define NUM_PREDICATELOCK_PARTITIONS (1 << LOG2_NUM_PREDICATELOCK_PARTITIONS)

I find this odd. Why is it a good idea to define the bufMapping
partitions like this? What's the explanation for the performance
numbers you saw?

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services