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: 1471.1339106082@sss.pgh.pa.us (view raw or flat)
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
reasons:

* 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
AllForks.

* 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

Responses

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-2014 The PostgreSQL Global Development Group