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: "k.jamison@fujitsu.com" <k.jamison@fujitsu.com>, 'Amit Kapila' <amit.kapila16@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Kyotaro Horiguchi <horikyota.ntt@gmail.com>, 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-15T11:11:26Z
Lists: pgsql-hackers
Attachments
- v13-Speedup-dropping-of-relation-buffers-during-recovery.patch (application/octet-stream) patch v13
Hi, > BTW, I think I see one problem in the code: > > > > if (RelFileNodeEquals(bufHdr->tag.rnode, rnode.node) && > > + bufHdr->tag.forkNum == forkNum[j] && tag.blockNum >= > > + bufHdr->firstDelBlock[j]) > > > > Here, I think you need to use 'i' not 'j' for forkNum and > > firstDelBlock as those are arrays w.r.t forks. That might fix the > > problem but I am not sure as I haven't tried to reproduce it. > > Thanks for advice. Right, that seems to be the cause of error, and fixing that > (using fork) solved the case. > I also followed the advice of Tsunakawa-san of using more meaningful > iterator Instead of using "i" & "j" for readability. > > I also added a new function when relation fork is bigger than the threshold > If (nblocks > BUF_DROP_FULLSCAN_THRESHOLD) > (DropRelFileNodeBuffersOfFork) Perhaps there's a better name for that > function. > However, as expected in the previous discussions, this is a bit slower than the > standard buffer invalidation process, because the whole shared buffers are > scanned nfork times. > Currently, I set the threshold to (NBuffers / 500) I made a mistake in the v12. I replaced the firstDelBlock[fork_num] with firstDelBlock[block_num], In the for-loop code block of block_num, because we want to process the current block of per-block loop OTOH, I used the firstDelBlock[fork_num] when relation fork is bigger than the threshold, or if the cached blocks of small relations were already invalidated. The logic could be either correct or wrong, so I'd appreciate feedback and comments/advice. Regards, Kirk Jamison
Commits
-
Fix size overflow in calculation introduced by commits d6ad34f3 and bea449c6.
- 519e4c9ee21a 14.0 landed
-
Optimize DropRelFileNodesAllBuffers() for recovery.
- bea449c635c0 14.0 landed
-
Optimize DropRelFileNodeBuffers() for recovery.
- d6ad34f3410f 14.0 landed
-
Cache smgrnblocks() results in recovery.
- c5315f4f4484 14.0 cited
-
Add a check to prevent overwriting valid data if smgrnblocks() gives a
- ffae5cc5a602 8.2.0 cited