Skip site navigation (1) Skip section navigation (2)

Re: slow dropping of tables, DropRelFileNodeBuffers, tas

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Simon Riggs <simon(at)2ndQuadrant(dot)com>
Cc: Sergey Koposov <koposov(at)ast(dot)cam(dot)ac(dot)uk>, Jeff Janes <jeff(dot)janes(at)gmail(dot)com>, Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: slow dropping of tables, DropRelFileNodeBuffers, tas
Date: 2012-06-07 21:54:42
Message-ID: (view raw or whole thread)
Lists: pgsql-hackers
I wrote:
> Simon Riggs <simon(at)2ndQuadrant(dot)com> writes:
>> Both of these, as attached up thread.
>> Simon's patch - dropallforks.v1.patch
>> Jeff's patch - DropRelFileNodeBuffers_unlock_v1.patch
>> (needs a little tidyup)

> OK, will take a look.

I didn't like dropallforks.v1.patch at all as presented, for several

* Introducing an AllForks notation that only works in some contexts is
a foot-gun of large caliber.  This concern is not academic: you broke
dropping of temp relations entirely, in the patch as presented, because
for temp rels DropRelFileNodeBuffers would hand off to
DropRelFileNodeLocalBuffers and the latter had not been taught about

* Since we have found out in this thread that the inner loop of
DropRelFileNodeBuffers is performance-critical for the cases we're
dealing with, it seems inappropriate to me to make its tests more
complex.  We want simpler, and we can have simpler given that the
relation-drop case cares neither about fork nor block number.

* The patch modified behavior of XLogDropRelation, which has not been
shown to be performance-critical, and probably can't be because the
hashtable it searches should never be all that large.  It certainly
doesn't matter to the speed of normal execution.

I thought it would be a lot safer and probably a little bit quicker
if we just split DropRelFileNodeBuffers into two routines, one for
the specific-fork case and one for the all-forks case; and then the
same for its main caller smgrdounlink.  So I modified the patch along
those lines and committed it.

As committed, the smgrdounlinkfork case is actually dead code; it's
never called from anywhere.  I left it in place just in case we want
it someday.

			regards, tom lane

In response to


pgsql-hackers by date

Next:From: Tom LaneDate: 2012-06-07 22:15:23
Subject: New Postgres committer: Kevin Grittner
Previous:From: Tom LaneDate: 2012-06-07 20:52:01
Subject: Re: slow dropping of tables, DropRelFileNodeBuffers, tas

Privacy Policy | About PostgreSQL
Copyright © 1996-2015 The PostgreSQL Global Development Group