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

[tsunakawa.ta...@fujitsu.com]

I looked at v14.


(1)
+               /* Get the total number of blocks for the supplied relation's 
fork */
+               for (j = 0; j < nforks; j++)
+               {
+                       BlockNumber             block = smgrnblocks(smgr_reln, 
forkNum[j]);
+                       nblocks += block;
+               }

Why do you sum all forks?


(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.
Why is BlockNumberIsValid(nblocks)) call needed?


(3)
        if (reln->smgr_cached_nblocks[forknum] == blocknum)
                reln->smgr_cached_nblocks[forknum] = blocknum + 1;
        else
+       {
+               /*
+                * DropRelFileNodeBuffers relies on the behavior that cached 
nblocks
+                * won't be invalidated by file extension while recovering.
+                */
+               Assert(!InRecovery);
                reln->smgr_cached_nblocks[forknum] = InvalidBlockNumber;
+       }

I think this change is not directly related to this patch and can be a separate 
patch, but I want to leave the decision up to a committer.


Regards
Takayuki Tsunakawa