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>, 'Thomas
Munro' <thomas.munro@gmail.com>, "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-11-18T09:04:49Z
Lists: pgsql-hackers
On Thursday, November 12, 2020 1:14 PM, Tsunakawa-san wrote: > The patch looks OK. I think as Thomas-san suggested, we can remove the > modification to smgrnblocks() and don't care wheter the size is cached or not. > But I think the current patch is good too, so I'd like to leave it up to a > committer to decide which to choose. > I measured performance in a different angle -- the time > DropRelFileNodeBuffers() and DropRelFileNodeAllBuffers() took. That > reveals the direct improvement and degradation. > > I used 1,000 tables, each of which is 1 MB. I used shared_buffers = 128 MB > for the case where the traditional full buffer scan is done, and shared_buffers > = 100 GB for the case where the optimization path takes effect. > > The results are almost good as follows: > > A. UNPATCHED > > 128 MB shared_buffers > 1. VACUUM = 0.04 seconds > 2. TRUNCATE = 0.04 seconds > > 100 GB shared_buffers > 3. VACUUM = 69.4 seconds > 4. TRUNCATE = 69.1 seconds > > > B. PATCHED > > 128 MB shared_buffers (full scan) > 5. VACUUM = 0.04 seconds > 6. TRUNCATE = 0.07 seconds > > 100 GB shared_buffers (optimized path) > 7. VACUUM = 0.02 seconds > 8. TRUNCATE = 0.08 seconds > > > So, I'd like to mark this as ready for committer. I forgot to reply. Thank you very much Tsunakawa-san for testing and to everyone who has provided their reviews and insights as well. Now thinking about smgrnblocks(), currently Thomas Munro is also working on implementing a shared SmgrRelation [1] to store sizes. However, since that is still under development and the discussion is still ongoing, I hope we can first commit these set of patches here as these are already in committable form. I think it's alright to accept the early improvements implemented in this thread to the source code. [1] https://www.postgresql.org/message-id/CA%2BhUKG%2B7Ok26MHiFWVEiAy2UMgHkrCieycQ1eFdA%3Dt2JTfUgwA%40mail.gmail.com 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