Hi, On Wed, Dec 11, 2024 at 02:56:08PM +0900, Michael Paquier wrote: > On Tue, Dec 10, 2024 at 02:16:14PM +0000, Bertrand Drouvot wrote: > > > > 2. One linked to PgStat_FunctionCounts pending stats, mentioning the use of > > memcmp() against zeroes to detect whether there are any pending stats: I > > think > > it has never been the case. > > * PgStat_FunctionCounts The actual per-function counts kept by a backend > * > - * This struct should contain only actual event counters, because we memcmp > - * it against zeroes to detect whether there are any pending stats. > + * This struct should contain only actual event counters, because pending > stats > + * always has non-zero content. > > pgstat_function_flush_cb() in pgstat_function.c says since > 5891c7a8ed8f, so that seems like a remnant from the original patch > that has introduced this inconsistency across its reviews: > "/* localent always has non-zero content */"
Thanks for looking at it! Yeah and same in pgstat_subscription_flush_cb(). > Your suggestion does not look completely right to me. There is > nothing preventing us from using something else than event counters > since we don't use memcpy() and there is no comparison work, no? It > seems to me that we could remove the entire sentence instead. Do you mean also remove the comments in pgstat_function_flush_cb() and pgstat_subscription_flush_cb()? Those comments look fine to me given the places where those pending entries are created meaning in pgstat_init_function_usage() for the functions and pgstat_report_subscription_error() and pgstat_report_subscription_conflict() for the subscriptions. Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
