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

From: Jamison, Kirk/ジャミソン カーク <k(dot)jamison(at)fujitsu(dot)com>
> I revised the patch based from my understanding of Horiguchi-san's comment,
> but I could be wrong.
> Quoting:
>
> "
> + /* Get the number of blocks for the supplied relation's
> fork */
> + nblocks = smgrnblocks(smgr_reln,
> forkNum[fork_num]);
> + Assert(BlockNumberIsValid(nblocks));
> +
> + if (nblocks < BUF_DROP_FULLSCAN_THRESHOLD)
>
> As mentioned upthread, the criteria whether we do full-scan or
> lookup-drop is how large portion of NBUFFERS this relation-drop can be
> going to invalidate. So the nblocks above should be the sum of number
> of blocks to be truncated (not just the total number of blocks) of all
> designated forks. Then once we decided to do lookup-drop method, we
> do that for all forks."

One takeaway from Horiguchi-san's comment is to use the number of blocks to invalidate for comparison, instead of all blocks in the fork. That is, use

nblocks = smgrnblocks(fork) - firstDelBlock[fork];

Does this make sense?

What do you think is the reason for summing up all forks? I didn't understand why. Typically, FSM and VM forks are very small. If the main fork is larger than NBuffers / 500, then v14 scans the entire shared buffers for the FSM and VM forks as well as the main fork, resulting in three scans in total.

Also, if you want to judge the criteria based on the total blocks of all forks, the following if should be placed outside the for loop, right? Because this if condition doesn't change inside the for loop.

+ if ((nblocks / (uint32)NBuffers) < BUF_DROP_FULLSCAN_THRESHOLD &&
+ BlockNumberIsValid(nblocks))
+ {

> > (2)
> > + if ((nblocks / (uint32)NBuffers) <
> > BUF_DROP_FULLSCAN_THRESHOLD &&
> > + BlockNumberIsValid(nblocks))
> > + {
> >
> > The division by NBuffers is not necessary, because both sides of = are
> > number of blocks.
>
> Again I based it from my understanding of the comment above,
> so nblocks is the sum of all blocks to be truncated for all forks.

But the left expression of "<" is a percentage, while the right one is a block count. Two different units are compared.

> > Why is BlockNumberIsValid(nblocks)) call needed?
>
> I thought we need to ensure that nblocks is not invalid, so I also added

When is it invalid? smgrnblocks() seems to always return a valid block number. Am I seeing a different source code (I saw HEAD)?

Regards
Takayuki Tsunakawa