Re: Inconsistency in determining the timestamp of the db statfile.

On Wed, Sep 9, 2020 at 9:11 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:

> On Wed, Sep 9, 2020 at 10:54 AM Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>
> wrote:
> >
> > On 2020/09/09 12:04, Amit Kapila wrote:
> > >
> > > No, before patch as well, if we can't read the DB entry say because
> > > the file is corrupt, we return true and use timestamp of global stats
> > > file and this is what is established by the original commit 187492b6.
> > > So, if we consider that as correct then the functionality is something
> > > like once we have read the timestamp of the global statfile, we use
> > > that if we can't read the actual db entry for whatever reason. It
> > > seems if we return false, the caller will call this function again in
> > > the loop. Now, I see the point that if we can't read some part of the
> > > file we should return false instead but not sure if that is helpful
> > > from the perspective of the caller of
> > > pgstat_read_db_statsfile_timestamp.
> >
> > When false is returned, the caller backend_read_statsfile() seems to
> > request the stats collector to write a fresh stats data into the file,
> > and then pgstat_read_db_statsfile_timestamp() can try to read the fresh
> > file that may not be corrupted. So returning false in that case seems ok
> > to me...
> >
>
> Hmm, the request to get fresh data is sent irrespective of true or
> false. We send it to get the latest data if it is not present and it
>

No we don't. Just before we request it, there is:
/* Normal acceptance case: file is not older than cutoff time */
if (ok && file_ts >= min_ts)
break;

So it only requests a new file in the case that it returned false.

--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/>
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>