Re: [Patch] Optimize dropping of relation buffers using dlist
Amit Kapila <amit.kapila16@gmail.com>
From: Amit Kapila <amit.kapila16@gmail.com>
To: "k.jamison@fujitsu.com" <k.jamison@fujitsu.com>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>, "tsunakawa.takay@fujitsu.com" <tsunakawa.takay@fujitsu.com>,
Andres Freund <andres@anarazel.de>, Tom Lane <tgl@sss.pgh.pa.us>, Thomas Munro <thomas.munro@gmail.com>, Robert Haas <robertmhaas@gmail.com>,
Tomas Vondra <tomas.vondra@2ndquadrant.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2020-12-23T08:51:08Z
Lists: pgsql-hackers
On Wed, Dec 23, 2020 at 1:07 PM k.jamison@fujitsu.com <k.jamison@fujitsu.com> wrote: > > On Tuesday, December 22, 2020 9:11 PM, Amit Kapila wrote: > > > In this code, I am slightly worried about the additional cost of each time > > checking smgrexists. Consider a case where there are many relations and only > > one or few of them have not cached the information, in such a case we will > > pay the cost of smgrexists for many relations without even going to the > > optimized path. Can we avoid that in some way or at least reduce its usage to > > only when it is required? One idea could be that we first check if the nblocks > > information is cached and if so then we don't need to call smgrnblocks, > > otherwise, check if it exists. For this, we need an API like > > smgrnblocks_cahced, something we discussed earlier but preferred the > > current API. Do you have any better ideas? > > Right. I understand the point that let's say there are 100 relations, and > the first 99 non-local relations happen to enter the optimization path, but the last > one does not, calling smgrexist() would be too costly and waste of time in that case. > The only solution I could think of as you mentioned is to reintroduce the new API > which we discussed before: smgrnblocks_cached(). > It's possible that we call smgrexist() only if smgrnblocks_cached() returns > InvalidBlockNumber. > So if everyone agrees, we can add that API smgrnblocks_cached() which will > Include the "cached" flag parameter, and remove the additional flag modifications > from smgrnblocks(). In this case, both DropRelFileNodeBuffers() and > DropRelFileNodesAllBuffers() will use the new API. > Yeah, let's do it that way unless anyone has a better idea to suggest. -- With Regards, Amit Kapila.
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