Re: shared-memory based stats collector - Mailing list pgsql-hackers

From Tomas Vondra
Subject Re: shared-memory based stats collector
Date
Msg-id 5253d750-890b-069b-031f-2a9b73e47832@2ndquadrant.com
Whole thread Raw
In response to Re: shared-memory based stats collector  (Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>)
Responses Re: shared-memory based stats collector
List pgsql-hackers
Hi,

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.

regards

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

Attachment

pgsql-hackers by date:

Previous
From: Mikhail Bautin
Date:
Subject: Resource cleanup callbacks for foreign data wrappers
Next
From: Amit Langote
Date:
Subject: Re: Speeding up INSERTs and UPDATEs to partitioned tables