On Mon, 17 Jun 2019 at 15:05, Tsunakawa, Takayuki
<tsunakawa.takay@jp.fujitsu.com> wrote:
> (1)
> +#define LOCKMETHODLOCALHASH_SHRINK_SIZE 64
>
> How about LOCKMETHODLOCALHASH_SHRINK_THRESHOLD, because this determines the threshold value to trigger shrinkage?
Codein PostgreSQL seems to use the term threshold.
That's probably better. I've renamed it to that.
> (2)
> +/* Complain if the above are not set to something sane */
> +#if LOCKMETHODLOCALHASH_SHRINK_SIZE < LOCKMETHODLOCALHASH_INIT_SIZE
> +#error "invalid LOCKMETHODLOCALHASH_SHRINK_SIZE"
> +#endif
>
> I don't think these are necessary, because these are fixed and not configurable. FYI, src/include/utils/memutils.h
doesn'thave #error to test these macros.
Yeah. I was thinking it was overkill when I wrote it, but somehow
couldn't bring myself to remove it. Done now.
> (3)
> + if (hash_get_num_entries(LockMethodLocalHash) == 0 &&
> + hash_get_max_bucket(LockMethodLocalHash) >
> + LOCKMETHODLOCALHASH_SHRINK_SIZE)
> + CreateLocalLockHash();
>
> I get an impression that Create just creates something where there's nothing. How about Init or Recreate?
Renamed to InitLocalLoclHash()
v5 is attached.
Thank you for the review.
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
