Re: shared-memory based stats collector

From: Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>
To: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>, andres(at)anarazel(dot)de
Cc: ah(at)cybertec(dot)at, magnus(at)hagander(dot)net, robertmhaas(at)gmail(dot)com, tgl(at)sss(dot)pgh(dot)pa(dot)us, pgsql-hackers(at)postgresql(dot)org
Subject: Re: shared-memory based stats collector
Date: 2018-10-30 00:49:59
Views: Raw Message | Whole Thread | Download mbox
Lists: pgsql-hackers


I've started looking at the patch over the past few days. I don't have
any deep insights at this point, but there seems to be some sort of
issue in pgstat_update_stat. When building using gcc, I do get this warning:

pgstat.c: In function ‘pgstat_update_stat’:
pgstat.c:648:18: warning: ‘now’ may be used uninitialized in this
function [-Wmaybe-uninitialized]
oldest_pending = now;
PostgreSQL installation complete.

which kinda makes sense, because 'now' is set only in the (!force)
branch. So if the very first call to pgstat_update_stat is with
force=true, it's not set, and the code executes this:

/* record oldest pending update time */
if (pgStatPendingTabHash == NULL)
oldest_pending = 0;
else if (oldest_pending == 0)
oldest_pending = now;

at which point we set "oldest_pending = now" with "now" containing some
random garbage.

When running this under valgrind, I get a couple of warnings in this
area of code - see the attached log with a small sample. Judging by the
locations I assume those are related to the same issue, but I have not
looked into that.


Tomas Vondra
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachment Content-Type Size
valgrind.log text/x-log 22.7 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Langote 2018-10-30 00:53:54 Re: Speeding up INSERTs and UPDATEs to partitioned tables
Previous Message Mikhail Bautin 2018-10-30 00:00:39 Resource cleanup callbacks for foreign data wrappers