On Wed, Sep 9, 2020 at 9:11 AM Amit Kapila <amit.kapil...@gmail.com> wrote:
> On Wed, Sep 9, 2020 at 10:54 AM Fujii Masao <masao.fu...@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 > 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/>