On Wed, Aug 24, 2011 at 03:38:09PM -0400, Tom Lane wrote:
> Merlin Moncure <mmonc...@gmail.com> writes:
> > On Wed, Aug 24, 2011 at 1:24 PM, Daniel Farina <dan...@heroku.com> wrote:
> >> At Heroku we use CREATE INDEX CONCURRENTLY with great success, but
> >> recently when frobbing around some indexes I realized that there is no
> >> equivalent for DROP INDEX, and this is a similar but lesser problem
> >> (as CREATE INDEX takes much longer), as DROP INDEX takes an ACCESS
> >> EXCLUSIVE lock on the parent table while doing the work to unlink
> >> files, which nominally one would think to be trivial, but I assure you
> >> it is not at times for even indexes that are a handful of gigabytes
> >> (let's say ~=< a dozen).
>
> > Are you sure that you are really waiting on the time to unlink the
> > file? there's other stuff going on in there like waiting for lock,
> > plan invalidation, etc. Point being, maybe the time consuming stuff
> > can't really be deferred which would make the proposal moot.
>
> Assuming the issue really is the physical unlinks (which I agree I'd
> like to see some evidence for), I wonder whether the problem could be
I'd believe it. With a cold cache (sudo sysctl -w vm.drop_caches=3), my
desktop takes 2.6s to "rm" a 985 MiB file.
> addressed by moving smgrDoPendingDeletes() to after locks are released,
> instead of before, in CommitTransaction/AbortTransaction. There does
> not seem to be any strong reason why we have to do that before lock
> release, since incoming potential users of a table should not be trying
> to access the old physical storage after that anyway.
Agreed. We now have $OLD_SUBJECT, but this is a win independently. I have
reviewed the code that runs between the old and new call sites, and I did not
identify a hazard of moving it as you describe.
nm
diff --git a/src/backend/access/transam/xact.c
b/src/backend/access/transam/xact.c
index 8f00186..8e4a455 100644
*** a/src/backend/access/transam/xact.c
--- b/src/backend/access/transam/xact.c
***************
*** 1944,1957 **** CommitTransaction(void)
*/
AtEOXact_Inval(true);
- /*
- * Likewise, dropping of files deleted during the transaction is best
done
- * after releasing relcache and buffer pins. (This is not strictly
- * necessary during commit, since such pins should have been released
- * already, but this ordering is definitely critical during abort.)
- */
- smgrDoPendingDeletes(true);
-
AtEOXact_MultiXact();
ResourceOwnerRelease(TopTransactionResourceOwner,
--- 1944,1949 ----
***************
*** 1961,1966 **** CommitTransaction(void)
--- 1953,1969 ----
RESOURCE_RELEASE_AFTER_LOCKS,
true, true);
+ /*
+ * Likewise, dropping of files deleted during the transaction is best
done
+ * after releasing relcache and buffer pins. (This is not strictly
+ * necessary during commit, since such pins should have been released
+ * already, but this ordering is definitely critical during abort.)
Since
+ * this may take many seconds, also delay until after releasing locks.
+ * Other backends will observe the attendant catalog changes and not
+ * attempt to access affected files.
+ */
+ smgrDoPendingDeletes(true);
+
/* Check we've released all catcache entries */
AtEOXact_CatCache(true);
***************
*** 2354,2360 **** AbortTransaction(void)
AtEOXact_Buffers(false);
AtEOXact_RelationCache(false);
AtEOXact_Inval(false);
- smgrDoPendingDeletes(false);
AtEOXact_MultiXact();
ResourceOwnerRelease(TopTransactionResourceOwner,
RESOURCE_RELEASE_LOCKS,
--- 2357,2362 ----
***************
*** 2362,2367 **** AbortTransaction(void)
--- 2364,2370 ----
ResourceOwnerRelease(TopTransactionResourceOwner,
RESOURCE_RELEASE_AFTER_LOCKS,
false, true);
+ smgrDoPendingDeletes(false);
AtEOXact_CatCache(false);
AtEOXact_GUC(false, 1);
***************
*** 4238,4250 **** AbortSubTransaction(void)
AtEOSubXact_RelationCache(false, s->subTransactionId,
s->parent->subTransactionId);
AtEOSubXact_Inval(false);
- AtSubAbort_smgr();
ResourceOwnerRelease(s->curTransactionOwner,
RESOURCE_RELEASE_LOCKS,
false, false);
ResourceOwnerRelease(s->curTransactionOwner,
RESOURCE_RELEASE_AFTER_LOCKS,
false, false);
AtEOXact_GUC(false, s->gucNestLevel);
AtEOSubXact_SPI(false, s->subTransactionId);
--- 4241,4253 ----
AtEOSubXact_RelationCache(false, s->subTransactionId,
s->parent->subTransactionId);
AtEOSubXact_Inval(false);
ResourceOwnerRelease(s->curTransactionOwner,
RESOURCE_RELEASE_LOCKS,
false, false);
ResourceOwnerRelease(s->curTransactionOwner,
RESOURCE_RELEASE_AFTER_LOCKS,
false, false);
+ AtSubAbort_smgr();
AtEOXact_GUC(false, s->gucNestLevel);
AtEOSubXact_SPI(false, s->subTransactionId);
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
