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

  1. Fix size overflow in calculation introduced by commits d6ad34f3 and bea449c6.

  2. Optimize DropRelFileNodesAllBuffers() for recovery.

  3. Optimize DropRelFileNodeBuffers() for recovery.

  4. Cache smgrnblocks() results in recovery.

  5. Add a check to prevent overwriting valid data if smgrnblocks() gives a