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

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

From: "tsunakawa.takay@fujitsu.com" <tsunakawa.takay@fujitsu.com>
To: "k.jamison@fujitsu.com" <k.jamison@fujitsu.com>
Cc: 'Amit Kapila' <amit.kapila16@gmail.com>, Kyotaro Horiguchi <horikyota.ntt@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>, Andres Freund <andres@anarazel.de>, Robert Haas <robertmhaas@gmail.com>, Tomas Vondra <tomas.vondra@2ndquadrant.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2020-09-29T01:51:12Z
Lists: pgsql-hackers
From: Jamison, Kirk/ジャミソン カーク <k.jamison@fujitsu.com>
> I also did not remove the duplicate code from smgrnblocks because Amit-san
> mentioned that when the caching for non-recovery cases is implemented, we
> can use it for non-recovery cases as well.

But the extra code is not used now.  The code for future usage should be added when it becomes necessary.  Duplicate code may make people think that you should add an argument to smgrnblocks() instead of adding a new function.

+		if (reln->smgr_cached_nblocks[forknum] != InvalidBlockNumber)
+			return reln->smgr_cached_nblocks[forknum];
+		else
+			return InvalidBlockNumber;

Anyway, the else block is redundant, as the variable contains InvalidBlockNumber.

Also, as Amit-san mentioned, the cause of the slight performance regression when shared_buffers is small needs to be investigated and addressed.  I think you can do it after sharing the performance result with a large shared_buffers.

I found no other problem.


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