On Fri, Sep 25, 2020 at 2:25 PM tsunakawa.ta...@fujitsu.com <tsunakawa.ta...@fujitsu.com> wrote: > > From: Amit Kapila <amit.kapil...@gmail.com> > > No, during recovery also we need to be careful. We need to ensure that > > we use cached value during recovery and cached value is always > > up-to-date. We can't rely on lseek and I have provided some scenario > > up thread [1] where such behavior can cause problem and then see the > > response from Tom Lane why the same can be true for recovery as well. > > > > The basic approach we are trying to pursue here is to rely on the > > cached value of 'number of blocks' (as that always gives correct value > > and even if there is a problem that will be our bug, we don't need to > > rely on OS for correct value and it will be better w.r.t performance > > as well). It is currently only possible during recovery so we are > > using it in recovery path and later once Thomas's patch to cache it > > for non-recovery cases is also done, we can use it for non-recovery > > cases as well. > > Although I may be still confused, I understood that Kirk-san's patch should: > > * Still focus on speeding up the replay of TRUNCATE during recovery. > > * During recovery, DropRelFileNodeBuffers() gets the cached size of the > relation fork. If it is cached, trust it and optimize the buffer > invalidation. If it's not cached, we can't trust the return value of > smgrnblocks() because it's the lseek(END) return value, so we avoid the > optimization. >
I agree with the above two points. > * Then, add a new function, say, smgrnblocks_cached() that simply returns the > cached block count, and DropRelFileNodeBuffers() uses it instead of > smgrnblocks(). > I am not sure if it worth adding a new function for this. Why not simply add a boolean variable in smgrnblocks for this? BTW, AFAICS, the latest patch doesn't have code to address this point. -- With Regards, Amit Kapila.
