On Wed, Sep 9, 2020 at 10:54 AM Fujii Masao <masao.fujii@oss.nttdata.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
is not guaranteed that the request will lead to a write of stats file.
So, I am not sure if we can override that with the corrupted file
case, sure there is something to it but if we really want to rely on
that mechanism we should explicitly send such a request on detection
of a corrupted file.
--
With Regards,
Amit Kapila.