> Please note that this patch does not apply anymore. Could you rebase, > please?
Done.
I also addressed some of Bertrand's points.
> === 1
>
> I wonder if this should use
> dshash_find_or_insert_extended(..., DSHASH_INSERT_NO_OOM)
> and release the local entry if it returns NULL?
Done. The insertion paths now use
dshash_find_or_insert_extended(..., DSHASH_INSERT_NO_OOM). In
pgstat_get_entry_ref(), the local entry is released if insertion returns NULL.
> === 4
>
> Current master provides dshash_get_dsa_area() since 762e329e83f, so this one
> looks now redundant.
Done. pgstat_get_dsa_for_kind() now uses
dshash_get_dsa_area(pgstat_get_hash_for_kind(kind))
> === 5
>
> Could pgstat_reset_entries_of_kind() scan only the hash returned by
> pgstat_get_hash_for_kind(kind)?
Done. I added pgstat_reset_matching_entries_in_hash().
pgstat_reset_matching_entries() still scans all hashes by calling that helper
for each hash. pgstat_reset_entries_of_kind() now scans only
pgstat_get_hash_for_kind(kind), instead of scanning all hashes and filtering by
kind.
> === 6
>
> I think the branch should be removed so pgstat_register_kind() reports the
> intended prerequisite error.
Done.
For the other points.
> === 2
>
> I wonder if this case should discard the statistics that no longer fit rather
> than prevent the server from starting?
I left this unchanged for now. Discarding persisted stats instead of failing
startup changes the failure semantics, so I think this needs more discussion.
I also think the existing behavior is sane: either the stats file is loaded, or
it is discarded as a whole.
> === 3
>
> That would retain independent iteration and hash partition locks while
> avoiding
> an additional shared memory allocation and per backend DSA state when
> independent
> memory accounting is not needed.
I kept own_hash tied to a dedicated DSA for now. I am not sure we would want to
support that mix and match behavior. A stats kind that asks for its own hash is
already asking to be isolated from the shared stats table, and using a dedicated
DSA keeps the hash storage and entry storage isolated in the same way.
I have not thought of a good use case for splitting those controls, but perhaps
there is one.
Attached is v2
--
Sami Imseih
Amazon Web Services AWS
v2-0001-pgstat-allow-a-stats-kind-to-use-its-own-dedicate.patch
Description: Binary data
