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: "tsunakawa.takay@fujitsu.com" <tsunakawa.takay@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-24T08:47:06Z
Lists: pgsql-hackers

Attachments

On Thursday, September 24, 2020 1:27 PM, Tsunakawa-san wrote:

> (1)
> +				for (cur_blk = firstDelBlock[j]; cur_blk <
> nblocks; cur_blk++)
> 
> The right side of "cur_blk <" should not be nblocks, because nblocks is not
> the number of the relation fork anymore.

Right. Fixed. It should be the total number of (n)blocks of relation.

> (2)
> +			BlockNumber		nblocks;
> +			nblocks = smgrnblocks(smgr_reln, forkNum[j]) -
> firstDelBlock[j];
> 
> You should either:
> 
> * Combine the two lines into one: BlockNumber nblocks = ...;
> 
> or
> 
> * Put an empty line between the two lines to separate declarations and
> execution statements.

Right. I separated them in the updated patch. And to prevent confusion,
instead of nblocks, nTotalBlocks & nBlocksToInvalidate are used.

/* Get the total number of blocks for the supplied relation's fork */
nTotalBlocks = smgrnblocks(smgr_reln, forkNum[j]);

/* Get the total number of blocks to be invalidated for the specified fork */
nBlocksToInvalidate = nTotalBlocks - firstDelBlock[j];
 

> After correcting these, I think you can check the recovery performance.

I'll send performance measurement results in the next email. Thanks a lot for the reviews!

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