Horton

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

tsunakawa.takay@fujitsu.com <tsunakawa.takay@fujitsu.com>

View thread View on postgr.es
From: "tsunakawa.takay@fujitsu.com" <tsunakawa.takay@fujitsu.com>
To: "k.jamison@fujitsu.com" <k.jamison@fujitsu.com>
Cc: 'Kyotaro Horiguchi' <horikyota.ntt@gmail.com>, "amit.kapila16@gmail.com" <amit.kapila16@gmail.com>, "tgl@sss.pgh.pa.us" <tgl@sss.pgh.pa.us>, "andres@anarazel.de" <andres@anarazel.de>, "robertmhaas@gmail.com" <robertmhaas@gmail.com>, "tomas.vondra@2ndquadrant.com" <tomas.vondra@2ndquadrant.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2020-09-23T02:26:13Z
Lists: 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 up to a committer.


Regards
Takayuki Tsunakawa

Commits

  1. Fix size overflow in calculation introduced by commits d6ad34f3 and bea449c6.

  2. Optimize DropRelFileNodesAllBuffers() for recovery.

  3. Optimize DropRelFileNodeBuffers() for recovery.

  4. Cache smgrnblocks() results in recovery.

  5. Add a check to prevent overwriting valid data if smgrnblocks() gives a