Re: [Patch] Optimize dropping of relation buffers using dlist
Amit Kapila <amit.kapila16@gmail.com>
From: Amit Kapila <amit.kapila16@gmail.com>
To: "tsunakawa.takay@fujitsu.com" <tsunakawa.takay@fujitsu.com>
Cc: Andres Freund <andres@anarazel.de>,
"k.jamison@fujitsu.com" <k.jamison@fujitsu.com>, Kyotaro Horiguchi <horikyota.ntt@gmail.com>,
"tgl@sss.pgh.pa.us" <tgl@sss.pgh.pa.us>, Thomas Munro <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-12-22T02:54:55Z
Lists: pgsql-hackers
On Tue, Dec 22, 2020 at 8:18 AM tsunakawa.takay@fujitsu.com <tsunakawa.takay@fujitsu.com> wrote: > > From: Amit Kapila <amit.kapila16@gmail.com> > > Why would all client backends wait for AccessExclusive lock on this > > relation? Say, a client needs a buffer for some other relation and > > that might evict this buffer after we release the lock on the > > partition. In StrategyGetBuffer, it is important to either have a pin > > on the buffer or the buffer header itself must be locked to avoid > > getting picked as victim buffer. Am I missing something? > > Ouch, right. (The year-end business must be making me crazy...) > > So, there are two choices here: > > 1) The current patch. > 2) Acquire the buffer header spinlock before releasing the buffer mapping lwlock, and eliminate the buffer tag comparison as follows: > > BufTableLookup(); > LockBufHdr(); > LWLockRelease(); > InvalidateBuffer(); > > I think both are okay. If I must choose either, I kind of prefer 1), because LWLockRelease() could take longer time to wake up other processes waiting on the lwlock, which is not very good to do while holding a spinlock. > > I also prefer (1). I will add some comments about the locking protocol in the next version of the patch. -- 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