Composing my rather long-winded response to Simon got me thinking -- which just led me to realize there is probably a need to fix another thing related to SSI. For correct serializable behavior in the face of concurrent DDL execution, I think that a request for a heavyweight ACCESS EXCLUSIVE lock might need to block until all SIREAD locks on the relation have been released. Picture, for example, what might happen if one transaction acquires some predicate locks, then commits (releasing its heavyweight lock on the table), and before concurrent READ WRITE transactions complete there is a CLUSTER on the table. Or a DROP INDEX. :-( Both require an ACCESS EXCLUSIVE lock. Since an active transaction would already have an ACCESS SHARE lock when acquiring the SIREAD locks, this couldn't block in the other direction or with an active transaction. That means that it couldn't cause any deadlocks if we added blocking to the acquisition of an ACCESS EXCLUSIVE based on this. If we don't do this I don't think that there is a more serious impact than inaccurate conflict detection for serializable transactions which are active when these operations are performed. Well, that and the possibility of seeing SIRead locks in the pg_locks view for indexes or tables which no longer exist. So far I don't see any crash modes or effects on non-serializable transactions. If this change is too destabilizing for this point in the release we could document it as a limitation and fix it in 9.2. We're pretty aggressive about cleaning up SIREAD locks as soon as allowable after transaction completion, so this probably wouldn't happen very often. -Kevin
On 27.04.2011 22:59, Kevin Grittner wrote: > For correct serializable behavior in the face of concurrent DDL > execution, I think that a request for a heavyweight ACCESS EXCLUSIVE > lock might need to block until all SIREAD locks on the relation have > been released. Picture, for example, what might happen if one > transaction acquires some predicate locks, then commits (releasing > its heavyweight lock on the table), and before concurrent READ WRITE > transactions complete there is a CLUSTER on the table. Or a DROP > INDEX. :-( Hmm, could we upgrade all predicate locks to relation-level predicate locks instead? -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> wrote: > On 27.04.2011 22:59, Kevin Grittner wrote: >> For correct serializable behavior in the face of concurrent DDL >> execution, I think that a request for a heavyweight ACCESS >> EXCLUSIVE lock might need to block until all SIREAD locks on the >> relation have been released. Picture, for example, what might >> happen if one transaction acquires some predicate locks, then >> commits (releasing its heavyweight lock on the table), and before >> concurrent READ WRITE transactions complete there is a CLUSTER on >> the table. Or a DROP INDEX. :-( > > Hmm, could we upgrade all predicate locks to relation-level > predicate locks instead? Tied to what backend? This only comes into play with transactions which have committed and are not yet able to release their predicate locks because of overlapping READ WRITE serializable transactions which are still active. Until that point the ACCESS SHARE lock (or stronger) is still there protecting against this. One way we could push this entirely into the heavyweight locking system would be for a committing transaction with predicate locks to somehow cause all overlapping read write serializable transactions which are still active to acquire an ACCESS SHARE lock on each relation for which the committing transaction has any predicate lock(s). (Unless of course they already hold some lock on a relevant relation.) This would need to be done before the committing transaction released its lock on the relation (which it must have acquired to read something which would cause it to have a predicate lock). Is that feasible? It seems like a lot of work, and I'm not sure how one transaction would convince another process's transaction to acquire the locks. As an alternative, perhaps we could find a way to leave the relation locks for a serializable transaction until it's safe to clean up the predicate locks? They could be downgraded to ACCESS SHARE if they are stronger. They would need to survive beyond not only the commit of the transaction, but also the termination of the connection. They would need to be immune to being chosen as deadlock victims. Any other ideas? -Kevin
On Wed, Apr 27, 2011 at 02:59:19PM -0500, Kevin Grittner wrote: > For correct serializable behavior in the face of concurrent DDL > execution, I think that a request for a heavyweight ACCESS EXCLUSIVE > lock might need to block until all SIREAD locks on the relation have > been released. Picture, for example, what might happen if one I think you're correct about this, but also agree that it would be reasonable to document the limitation for now and punt on a fix until 9.2. On Wed, Apr 27, 2011 at 04:09:38PM -0500, Kevin Grittner wrote: > Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> wrote: > > Hmm, could we upgrade all predicate locks to relation-level > > predicate locks instead? > > Tied to what backend? This only comes into play with transactions > which have committed and are not yet able to release their predicate > locks because of overlapping READ WRITE serializable transactions > which are still active. Until that point the ACCESS SHARE lock (or > stronger) is still there protecting against this. I think Heikki was suggesting to upgrade to relation-level SIREAD locks. This seems like it would work, but might cause a lot of aborts immediately afterwards. I do wonder if it might be necessary to upgrade index locks to relation-level locks on the heap relation, in cases of dropping the index. > One way we could push this entirely into the heavyweight locking > system would be for a committing transaction with predicate locks to > somehow cause all overlapping read write serializable transactions > which are still active to acquire an ACCESS SHARE lock on each > relation for which the committing transaction has any predicate > lock(s). I'm not entirely clear on how this would work, but it sounds like it could also have a significant performance cost. > As an alternative, perhaps we could find a way to leave the relation > locks for a serializable transaction until it's safe to clean up the > predicate locks? They could be downgraded to ACCESS SHARE if they > are stronger. They would need to survive beyond not only the commit > of the transaction, but also the termination of the connection. > They would need to be immune to being chosen as deadlock victims. This sounds like it would require major changes to the heavyweight lock manager. There's already a notion of keeping locks past backend termination for 2PC prepared transactions, but it would be hard to apply here. The most practical solutions I see here are to, when acquiring an AccessExclusive lock, either wait for any associated SIREAD locks to go away, or to promote them to relation level. Dan -- Dan R. K. Ports MIT CSAIL http://drkp.net/
Dan Ports <drkp(at)csail(dot)mit(dot)edu> wrote: > On Wed, Apr 27, 2011 at 04:09:38PM -0500, Kevin Grittner wrote: >> Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> wrote: >>> Hmm, could we upgrade all predicate locks to relation-level >>> predicate locks instead? >> >> Tied to what backend? > I think Heikki was suggesting to upgrade to relation-level SIREAD > locks. Oh, yeah. That *is* what he said, isn't it. That's a simpler solution which works just fine. Please forget my over-excited flights of fancy on the topic -- I was so focused on what I thought was the solution I misread Heikki's much better idea as some variation on my theme. :-( -Kevin
On Wed, Apr 27, 2011 at 8:59 PM, Kevin Grittner <Kevin(dot)Grittner(at)wicourts(dot)gov> wrote: > For correct serializable behavior in the face of concurrent DDL > execution, I think that a request for a heavyweight ACCESS EXCLUSIVE > lock might need to block until all SIREAD locks on the relation have > been released. Picture, for example, what might happen if one > transaction acquires some predicate locks, then commits (releasing > its heavyweight lock on the table), and before concurrent READ WRITE > transactions complete there is a CLUSTER on the table. Or a DROP > INDEX. :-( Sorry, I can't picture it. What will happen? > Both require an ACCESS EXCLUSIVE lock. Since an active transaction > would already have an ACCESS SHARE lock when acquiring the SIREAD > locks, this couldn't block in the other direction or with an active > transaction. That means that it couldn't cause any deadlocks if we > added blocking to the acquisition of an ACCESS EXCLUSIVE based on > this. > > If we don't do this I don't think that there is a more serious > impact than inaccurate conflict detection for serializable > transactions which are active when these operations are performed. > Well, that and the possibility of seeing SIRead locks in the > pg_locks view for indexes or tables which no longer exist. So far I > don't see any crash modes or effects on non-serializable > transactions. If this change is too destabilizing for this point in > the release we could document it as a limitation and fix it in 9.2. I don't think this should wait for 9.2 It either works, or it doesn't. Putting caveats in there will just detract from people's belief in it. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Simon Riggs <simon(at)2ndQuadrant(dot)com> wrote: > On Wed, Apr 27, 2011 at 8:59 PM, Kevin Grittner > <Kevin(dot)Grittner(at)wicourts(dot)gov> wrote: > >> For correct serializable behavior in the face of concurrent DDL >> execution, I think that a request for a heavyweight ACCESS >> EXCLUSIVE lock might need to block until all SIREAD locks on the >> relation have been released. Picture, for example, what might >> happen if one transaction acquires some predicate locks, then >> commits (releasing its heavyweight lock on the table), and before >> concurrent READ WRITE transactions complete there is a CLUSTER on >> the table. Or a DROP INDEX. :-( > > Sorry, I can't picture it. What will happen? Rather than get into a complex generalized discussion, I'll provide the simplest example I can picture. Let's say we have two concurrent transactions, T0 and T1. Up to this point T0 has read from table x and written to table y based on what was read from x. T1 has read from y -- but since the transactions are concurrent, it doesn't see T0's write. Let's assume each read was of a single tuple accessed through a btree index, so each transaction has one tuple lock on the heap and one page lock on the index. Now T0 commits. T0 must hold its SIREAD locks because of concurrent transaction T1. Everything is fine so far. Now a DBA runs CLUSTER against table x. The SIREAD locks held by T0 are probably now wrong, because the tuple and its index entry are likely to have moved. Now T1 writes to table x based on what it read from y. It could incorrectly detect a conflict if it happens to write to a tuple at the locked block and tuple number when it's not the same row. Worse, it could miss detecting a conflict if it's really updating the same row that T0 wrote, and that's not detected because it's not at the locked location any more. >> If this change is too destabilizing for this point in the release >> we could document it as a limitation and fix it in 9.2. > > I don't think this should wait for 9.2 > > It either works, or it doesn't. Putting caveats in there will just > detract from people's belief in it. I see your point. And this clearly is a bug. We failed to consider this category of problem and cover it. Heikki's suggestion is clearly the best plan. In the example above, when the CLUSTER was run it would make a call to the predicate locking module telling it to promote all SIREAD locks for table x or any of its indexes into a relation level lock on table x. The CLUSTER would cause us to lose the finer granularity of the locks on the table, and in this example if T1 wrote to table x it be rolled back with a serialization failure. This could be a false positive, but we expect to have some of those -- the transaction is retried and then succeeds. You can't have a false negative, so integrity is preserved. I'll try to work up a detailed plan of which commands need what actions. For example, DROP INDEX needs to promote SIREAD locks on the dropped index to relation locks on the related table. TRUNCATE TABLE is a little confusing -- I think that if it's run in a serializable transaction we generate a rw-conflict out from that transaction to every transaction holding any SIREAD lock on that table or any of its indexes, and then clear those SIREAD locks. This'll take some study. -Kevin
"Kevin Grittner" <Kevin(dot)Grittner(at)wicourts(dot)gov> wrote: > This'll take some study. I've gone through the list of commands in the development docs with an eye toward exposing anything else we might have missed in dealing with the SSI predicate locking. Some of this needs further research, but I'm posting what I have so far so that people have a chance to argue about what I think needs doing or to point out anything else they can think of which I'm still missing. And, of course, so people know where we are with it. I tested file_fdw at the serializable isolation level and found that no predicate locks were taken and no problems showed up. I think that's all correct -- I don't see how we can expect to include FDW data in serializability. I haven't dug into ALTER INDEX enough to know whether it can ever cause an index to be rebuilt. If so, we need to treat it like DROP INDEX and REINDEX -- which should change all predicate locks of any granularity on the index into relation locks on the associated table. CLUSTER or an ALTER TABLE which causes a rewrite should change all predicate locks on the table and all indexes into relation locks on the associated table. (Obviously, an existing relation lock on the table doesn't require any action.) TRUNCATE TABLE and DROP TABLE should generate a rw-conflict *in* to the enclosing transaction (if it is serializable) from all transactions holding predicate locks on the table or its indexes. Note that this could cause a transactions which is running one of these statements to roll back with a serialization error. This seems correct to me, since these operations essentially delete all rows. If you don't want the potential rollback, these operations should be run at another isolation level. The difference between these two statements is that I think that TRUNCATE TABLE should also move the existing predicate locks to relation locks on the table while DROP TABLE (for obvious reasons) should just delete the predicate locks. DROP DATABASE should quietly clean up any predicate locks from committed transactions which haven't yet hit their cleanup point because of overlapping transactions in other databases. DROP OWNED or DROP SCHEMA could CASCADE to some of the above, in which case the above rules would apply. I need to do some testing with Large Objects to see what state those are in with SSI, unless someone else has already looked at that. SAVEPOINTs have been considered and there is a lot of coding to try to handle them correctly, but they probably merit a bit more attention and testing. On some quick testing everything seemed to be in line with previous discussions and with what seems logical to me, but our SSI regression tests in src/test/isolation lack any coverage for them and I don't know how much others have beat up on them. At a minimum we should add a couple tests. I couldn't find anything else which hadn't already been considered and covered. More to come as I finish investigating. -Kevin
"Kevin Grittner" <Kevin(dot)Grittner(at)wicourts(dot)gov> wrote: > I haven't dug into ALTER INDEX enough to know whether it can ever > cause an index to be rebuilt. If so, we need to treat it like > DROP INDEX and REINDEX -- which should change all predicate locks > of any granularity on the index into relation locks on the > associated table. > > CLUSTER or an ALTER TABLE which causes a rewrite should change all > predicate locks on the table and all indexes into relation locks > on the associated table. (Obviously, an existing relation lock on > the table doesn't require any action.) > > TRUNCATE TABLE and DROP TABLE should generate a rw-conflict *in* > to the enclosing transaction (if it is serializable) from all > transactions holding predicate locks on the table or its indexes. > Note that this could cause a transactions which is running one of > these statements to roll back with a serialization error. This > seems correct to me, since these operations essentially delete all > rows. If you don't want the potential rollback, these operations > should be run at another isolation level. The difference between > these two statements is that I think that TRUNCATE TABLE should > also move the existing predicate locks to relation locks on the > table while DROP TABLE (for obvious reasons) should just delete > the predicate locks. > > DROP DATABASE should quietly clean up any predicate locks from > committed transactions which haven't yet hit their cleanup point > because of overlapping transactions in other databases. I missed VACUUM FULL when pulling together the above, but I haven't found any other omissions. (Still happy to hear about any that anyone can spot.) I notice that most of these need to shift transfer locks to relation locks on the heap, either for a single index or for the heap and all indexes. I wrote a function to do this and called it from one place to be able to test it. Consider this a WIP patch on which I would appreciate review while I work on finding the other places to call it and the miscellaneous other fixes needed. Note that I had to expose one previously-static function from index.c to find the heap OID from the index OID. Also, I ran pgindent against predicate.c, as I generally like to do when I modify much code, and it found four comment blocks in predicate.c touched since the recent global pgindent run which it re-wrapped. I can manually exclude those from the final patch if people would prefer that; but if people can ignore those whitespace tweaks, it might not be all bad to get standard formatting onto them at this point. -Kevin
Just a quick status update. I wrote: > Consider this a WIP patch The serializable branch on my git repo has a modified form of this which has been tested successfully with: DROP INDEX REINDEX VACUUM FULL CLUSTER ALTER TABLE I'm holding off on posting another version of the patch until I cover the remaining commands, which are: TRUNCATE TABLE DROP TABLE DROP DATABASE I'm having to work on this off-hours at the moment, so expect the patch to come sometime this weekend. -Kevin
On 30.04.2011 01:04, Kevin Grittner wrote: > TRUNCATE TABLE and DROP TABLE should generate a rw-conflict *in* to > the enclosing transaction (if it is serializable) from all > transactions holding predicate locks on the table or its indexes. > Note that this could cause a transactions which is running one of > these statements to roll back with a serialization error. This seems > correct to me, since these operations essentially delete all rows. > If you don't want the potential rollback, these operations should be > run at another isolation level. The difference between these two > statements is that I think that TRUNCATE TABLE should also move the > existing predicate locks to relation locks on the table while DROP > TABLE (for obvious reasons) should just delete the predicate locks. Note that TRUNCATE has never been MVCC-safe anyway. Perhaps it's best to just treat it like DROP TABLE. Or can we use the predicate lock mechanism to abort serializable transactions that incorrectly see the table as empty? > DROP DATABASE should quietly clean up any predicate locks from > committed transactions which haven't yet hit their cleanup point > because of overlapping transactions in other databases. This is just an optimization, right? The predicate locks will eventually go away anyway. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
On 1 May 2011 I wrote: > Consider this a WIP patch Just so people know where this stands... By 8 May 2011 I had the attached. I didn't post it because I was not confident I had placed the calls to the SSI functions for DROP TABLE and TRUNCATE TABLE correctly. Then came PGCon and also the row versus tuple discussion -- the patch for which had conflicts with this one. I will be reviewing this on the weekend and making any final adjustments, but the patch is very likely to look just like this with the possible exception of placement of the DROP TABLE and TRUNCATE TABLE calls. If anyone wants to start reviewing this now, it would leave more time for me to make changes if needed. Also, if anyone has comments or hints about the placement of those calls, I'd be happy to receive them. This patch compiles with `make world`, passes `make check-world`, and passes both `make installcheck-world` and `make -C src/test/isolation installcheck` against a database with default_transaction_isolation = 'serializable'. It also passed a few ad hoc tests of the implemented functionality. -Kevin
On 03.06.2011 21:04, Kevin Grittner wrote: > Also, if anyone has comments or hints about the placement of those > calls, I'd be happy to receive them. heap_drop_with_catalog() schedules the relation for deletion at the end of transaction, but it's still possible that the transaction aborts and the heap doesn't get dropped after all. If you put the DropAllPredicateLocksFromTable() call there, and the transaction later aborts, you've lost all the locks already. I think you'll need to just memorize the lock deletion command in a backend-local list, and perform the deletion in a post-commit function. Something similar to the PendingRelDelete stuff in storage.c. In fact, hooking into smgrDoPendingDeletes would work, but that seems like a modularity violation. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> wrote: > On 03.06.2011 21:04, Kevin Grittner wrote: >> Also, if anyone has comments or hints about the placement of >> those calls, I'd be happy to receive them. > > heap_drop_with_catalog() schedules the relation for deletion at > the end of transaction, but it's still possible that the > transaction aborts and the heap doesn't get dropped after all. If > you put the DropAllPredicateLocksFromTable() call there, and the > transaction later aborts, you've lost all the locks already. > > I think you'll need to just memorize the lock deletion command in > a backend-local list, and perform the deletion in a post-commit > function. Something similar to the PendingRelDelete stuff in > storage.c. In fact, hooking into smgrDoPendingDeletes would work, > but that seems like a modularity violation. Thanks. That's helpful. Will look at that code and do something similar. I knew it didn't look right in the place it was, but couldn't quite see what to do instead.... -Kevin
"Kevin Grittner" <Kevin(dot)Grittner(at)wicourts(dot)gov> writes: > Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> wrote: >> I think you'll need to just memorize the lock deletion command in >> a backend-local list, and perform the deletion in a post-commit >> function. Something similar to the PendingRelDelete stuff in >> storage.c. In fact, hooking into smgrDoPendingDeletes would work, >> but that seems like a modularity violation. > Thanks. That's helpful. Will look at that code and do something > similar. Keep in mind that it's too late to throw any sort of error post-commit. Any code you add there will need to have negligible probability of failure. regards, tom lane
Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote: > "Kevin Grittner" <Kevin(dot)Grittner(at)wicourts(dot)gov> writes: >> Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> wrote: >>> I think you'll need to just memorize the lock deletion command >>< in a backend-local list, and perform the deletion in a >>> post-commit function. Something similar to the PendingRelDelete >>> stuff in storage.c. In fact, hooking into smgrDoPendingDeletes >>> would work, but that seems like a modularity violation. > >> Thanks. That's helpful. Will look at that code and do something >> similar. > > Keep in mind that it's too late to throw any sort of error > post-commit. Any code you add there will need to have negligible > probability of failure. Thanks for the heads-up. I think we're OK in that regard, though. We have two commit-time functions in SSI: PreCommit_CheckForSerializationFailure(void) which is the "last chance for failure" before actual commit, and ReleasePredicateLocks(const bool isCommit) which is for resource cleanup after rollback or commit is effective. We pretty much can't fail on the latter except for Assert statements, and this new logic would be the same. It's hard to fail when freeing resources unless something is quite seriously hosed already. This is where I was planning to put the freeing of the locks, based on queuing up the request at the time the DROP TABLE statement is encountered. -Kevin
Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> wrote: > I think you'll need to just memorize the lock deletion command in > a backend-local list, and perform the deletion in a post-commit > function. Hmm. As mentioned earlier in the thread, cleaning these up doesn't actually have any benefit beyond freeing space in the predicate locking collections. I'm not sure that benefit is enough to justify this much new mechanism. Maybe I should just leave them alone and let them get cleaned up in due course with the rest of the locks. Any opinions on that? -Kevin
On 03.06.2011 23:44, Kevin Grittner wrote: > Heikki Linnakangas<heikki(dot)linnakangas(at)enterprisedb(dot)com> wrote: > >> I think you'll need to just memorize the lock deletion command in >> a backend-local list, and perform the deletion in a post-commit >> function. > > Hmm. As mentioned earlier in the thread, cleaning these up doesn't > actually have any benefit beyond freeing space in the predicate > locking collections. I'm not sure that benefit is enough to justify > this much new mechanism. Maybe I should just leave them alone and > let them get cleaned up in due course with the rest of the locks. > Any opinions on that? Is there a chance of false positives if oid wraparound happens and a new table gets the same oid as the old one? It's also possible for a heap to get the OID of an old index or vice versa, will that confuse things? -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> wrote: > On 03.06.2011 23:44, Kevin Grittner wrote: >> Hmm. As mentioned earlier in the thread, cleaning these up >> doesn't actually have any benefit beyond freeing space in the >> predicate locking collections. I'm not sure that benefit is >> enough to justify this much new mechanism. Maybe I should just >> leave them alone and let them get cleaned up in due course with >> the rest of the locks. Any opinions on that? > > Is there a chance of false positives if oid wraparound happens and > a new table gets the same oid as the old one? It's also possible > for a heap to get the OID of an old index or vice versa, will that > confuse things? Tuple locks should be safe from that because we use the tuple xmin as part of the target key, but page and heap locks could result in false positive serialization failures on OID wraparound unless we do the cleanup. I guess that tips the scales in favor of it being worth the extra code. I think it's still in that gray area where it's a judgment call because it would be very infrequent and it would be recoverable; but the fewer mysterious rollbacks, the fewer posts about it we'll get. So any costs in maintaining the extra code will probably be saved in reduced support, even if the performance benefit is negligible. -Kevin
"Kevin Grittner" <Kevin(dot)Grittner(at)wicourts(dot)gov> wrote: > Tuple locks should be safe from that because we use the tuple xmin > as part of the target key, but page and heap locks That should have read "page and relation locks". > I guess that tips the scales in favor of it being worth the extra > code. I think it's still in that gray area I just thought of something which takes it out of the gray area for me: pg_locks. Even though it would be extremely rare for a false positive to actually occur if we let this go, people would be sure to get confused by seeing locks on the dropped objects in the pg_locks view.. They've got to be cleaned up. -Kevin
Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> writes: > On 03.06.2011 21:04, Kevin Grittner wrote: >> Also, if anyone has comments or hints about the placement of those >> calls, I'd be happy to receive them. > heap_drop_with_catalog() schedules the relation for deletion at the end > of transaction, but it's still possible that the transaction aborts and > the heap doesn't get dropped after all. If you put the > DropAllPredicateLocksFromTable() call there, and the transaction later > aborts, you've lost all the locks already. But on the third thought: is that wrong? Surely locks taken by an aborted transaction can be discarded. regards, tom lane
On 04.06.2011 19:19, Tom Lane wrote: > Heikki Linnakangas<heikki(dot)linnakangas(at)enterprisedb(dot)com> writes: >> On 03.06.2011 21:04, Kevin Grittner wrote: >>> Also, if anyone has comments or hints about the placement of those >>> calls, I'd be happy to receive them. > >> heap_drop_with_catalog() schedules the relation for deletion at the end >> of transaction, but it's still possible that the transaction aborts and >> the heap doesn't get dropped after all. If you put the >> DropAllPredicateLocksFromTable() call there, and the transaction later >> aborts, you've lost all the locks already. > > But on the third thought: is that wrong? Surely locks taken by an > aborted transaction can be discarded. These are predicate locks - there can be "locks" on the table belonging to transactions that have already committed. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com