RE: [Patch] Optimize dropping of relation buffers using dlist

On Tuesday, November 10, 2020 3:10 PM, Tsunakawa-san wrote:
> From: Jamison, Kirk/ジャミソン カーク <k.jamison@fujitsu.com>
> > So I proceeded to update the patches using the "cached" parameter and
> > updated the corresponding comments to it in 0002.
> 
> OK, I'm in favor of the name "cached" now, although I first agreed with
> Horiguchi-san in that it's better to use a name that represents the nature
> (accurate) of information rather than the implementation (cached).  Having
> a second thought, since smgr is a component that manages relation files on
> storage (file system), lseek(SEEK_END) is the accurate value for smgr.  The
> cached value holds a possibly stale size up to which the relation has
> extended.
> 
> 
> The patch looks almost good except for the minor ones:

Thank you for the review!

> (1)
> +extern BlockNumber smgrnblocks(SMgrRelation reln, ForkNumber
> forknum,
> +							   bool *accurate);
> 
> It's still accurate here.

Already fixed in 0002.

> (2)
> + *		This is only called in recovery when the block count of any
> fork is
> + *		cached and the total number of to-be-invalidated blocks per
> relation
> 
> count of any fork is
> -> counts of all forks are

Fixed in 0003/
 
> (3)
> In 0004, I thought you would add the invalidated block counts of all relations
> to determine if the optimization is done, as Horiguchi-san suggested.  But I
> find the current patch okay too.

Yeah, I found my approach easier to implement. The new change in 0004 is that
when entering the optimized path we now call FindAndDropRelFileNodeBuffers()
instead of DropRelFileNodeBuffers().

I have attached all the updated patches.
I'd appreciate your feedback.

Regards,
Kirk Jamison