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

k.jamison@fujitsu.com <k.jamison@fujitsu.com>

From: "k.jamison@fujitsu.com" <k.jamison@fujitsu.com>
To: 'Kyotaro Horiguchi' <horikyota.ntt@gmail.com>
Cc: "tsunakawa.takay@fujitsu.com" <tsunakawa.takay@fujitsu.com>, "andres@anarazel.de" <andres@anarazel.de>, "amit.kapila16@gmail.com" <amit.kapila16@gmail.com>, "tgl@sss.pgh.pa.us" <tgl@sss.pgh.pa.us>, "thomas.munro@gmail.com" <thomas.munro@gmail.com>, "robertmhaas@gmail.com" <robertmhaas@gmail.com>, "tomas.vondra@2ndquadrant.com" <tomas.vondra@2ndquadrant.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2020-11-27T02:19:57Z
Lists: pgsql-hackers
> From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
> Hello, Kirk. Thank you for the new version.

Hi, Horiguchi-san. Thank you for your very helpful feedback.
I'm updating the patches addressing those.

> +			if (!smgrexists(rels[i], j))
> +				continue;
> +
> +			/* Get the number of blocks for a relation's fork */
> +			blocks[i][numForks] = smgrnblocks(rels[i], j,
> NULL);
> 
> If we see a fork which its size is not cached we must give up this optimization
> for all target relations.

I did not use the "cached" flag in DropRelFileNodesAllBuffers and use InRecovery
when deciding for optimization because of the following reasons:
XLogReadBufferExtended() calls smgrnblocks() to apply changes to relation page
contents. So in DropRelFileNodeBuffers(), XLogReadBufferExtended() is called
during VACUUM replay because VACUUM changes the page content.
OTOH, TRUNCATE doesn't change the relation content, it just truncates relation pages
without changing the page contents. So XLogReadBufferExtended() is not called, and
the "cached" flag will always return false. I tested with "cached" flags before, and this
always return false, at least in DropRelFileNodesAllBuffers. Due to this, we cannot use
the cached flag in DropRelFileNodesAllBuffers(). However, I think we can still rely on
smgrnblocks to get the file size as long as we're InRecovery. That cached nblocks is still
guaranteed to be the maximum in the shared buffer.
Thoughts?

Regards,
Kirk Jamison



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