RE: [Patch] Optimize dropping of relation buffers using dlist
tsunakawa.takay@fujitsu.com <tsunakawa.takay@fujitsu.com>
From: "tsunakawa.takay@fujitsu.com" <tsunakawa.takay@fujitsu.com>
To: 'Amit Kapila' <amit.kapila16@gmail.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-22T01:42:55Z
Lists: pgsql-hackers
From: Amit Kapila <amit.kapila16@gmail.com> > This answers the second part of the question but what about the first > part (We hold a buffer partition lock, and have done a lookup in th > mapping table. Why are we then rechecking the > relfilenode/fork/blocknum?) > > I think we don't need such a check, rather we can have an Assert > corresponding to that if-condition in the patch. I understand it is > safe to compare relfilenode/fork/blocknum but it might confuse readers > of the code. Hmm, you're right. I thought someone else could steal the found buffer and use it for another block because the buffer mapping lwlock is released without pinning the buffer or acquiring the buffer header spinlock. However, in this case (replay of TRUNCATE during recovery), nobody steals the buffer: bgwriter or checkpointer doesn't use a buffer for a new block, and the client backend waits for AccessExclusive lock. > I have started doing minor edits to the patch especially planning to > write a theory why is this optimization safe and here is what I can > come up with: Thank you, that's fluent and easier to understand. Regards Takayuki Tsunakawa
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