RE: [Patch] Optimize dropping of relation buffers using dlist - Mailing list pgsql-hackers

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
upto a committer. 


Regards
Takayuki Tsunakawa