Thread: pgstat_read_statsfiles() and reset timestamp

pgstat_read_statsfiles() and reset timestamp

From
Fujii Masao
Date:
Hi,

pgstat_read_statsfiles() sets each stat_reset_timestamp to
the current timestamp, at the beginning of the function,
just in case we fail to load the existing statsfile. This code is
originally introduced by commit 4c468b37a2.

But commit ad1b5c842b changed pgstat_read_statsfiles() so that
the stats including stat_reset_timestamp are zeroed in that case,
so now there seems no need to set each stat_reset_timestamp.
Thought?

Attached is the patch that removes such unnecessary sets of
stat_reset_timestamp from pgstat_read_statsfiles().

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

Attachment

Re: pgstat_read_statsfiles() and reset timestamp

From
Tom Lane
Date:
Fujii Masao <masao.fujii@oss.nttdata.com> writes:
> pgstat_read_statsfiles() sets each stat_reset_timestamp to
> the current timestamp, at the beginning of the function,
> just in case we fail to load the existing statsfile. This code is
> originally introduced by commit 4c468b37a2.

> But commit ad1b5c842b changed pgstat_read_statsfiles() so that
> the stats including stat_reset_timestamp are zeroed in that case,
> so now there seems no need to set each stat_reset_timestamp.

Huh?  The zeroing happens before those fields are filled.

> Attached is the patch that removes such unnecessary sets of
> stat_reset_timestamp from pgstat_read_statsfiles().

-1, minus a lot actually.  What this will do is that if there's
no stats file, the reset timestamps will all read as whatever
our epoch timestamp is (2000-01-01, I think).  This is not a
corner case, either --- it's the expected path at first start.
We want current time to be used in that case.

If there are any code paths in pgstat_read_statsfiles that
re-zero these structs later, they need to be fixed to restore
the reset timestamps to these values, as well.

            regards, tom lane