|From:||Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>|
|Cc:||andres(at)anarazel(dot)de, 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|
|Views:||Raw Message | Whole Thread | Download mbox|
Hello. Thank you for looking this.
At Tue, 30 Oct 2018 01:49:59 +0100, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com> wrote in <5253d750-890b-069b-031f-2a9b73e47832(at)2ndquadrant(dot)com>
> 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
> 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.
Uggh! The reason for the code is "last_report = now" comes later
than the code around... Fixed.
> 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.
There was several typos/thinkos related to pointers modifed from
original variables. There was a code like the following in the
memset(&shared_globalStats, 0, siazeof(shared_globalStats));
It was not fixed despite this patch changes the type of the
variable from PgStat_GlboalStats to (PgStat_GlobalStats *). As
the result major part of the varialbe remaineduninitialized.
I re-ran this version on valgrind and I didn't see such kind of
problem. Thank you for the testing.
I refactored the patch into complete set consists of 8 files.
- dshash sequential scan feature
- dshash contitional lock feature
- Shared memory base stats collector.
- Remove stats collector process
- Change stats collector to shared memory base
(This needs 0004 to work, but it is currently separate from
this for readability)
- Archiver process needs to touch shared memory.
(I didn't check EXEC_BACKEND case)
- I removed pg_stat_tmp directory so move pg_stat_statements
file stored there to pg_stat directory. (This would need fix)
- For the same reason with 0005.
- Removes description related to pg_stat_tmp.
- Just refactoring. Splits the current postmaster/pgstat.c into
two files statmon/pgstat.c and statmon/bestatus.c.
NTT Open Source Software Center
|Next Message||David Rowley||2018-11-08 11:54:51||Re: PostgreSQL Limits and lack of documentation about them.|
|Previous Message||Kyotaro HORIGUCHI||2018-11-08 11:42:22||Re: shared-memory based stats collector|