2009/10/25 Simon Riggs <si...@2ndquadrant.com>: > On Mon, 2009-10-19 at 17:48 +0100, Dean Rasheed wrote: > >> This is a WIP patch to replace the after-trigger queues with TID bitmaps >> to prevent them from using excessive amounts of memory. Each round of >> trigger executions is a modified bitmap heap scan. > > This is an interesting patch. The justification is fine, the idea is > good, though I'd like to see more analysis of the technique, what other > options exist and some thought about when we should use the technique. > > We have a bitmap for each UPDATE statement, I think, but there's no docs > or readme. Why just UPDATE? Is the cost of starting up the bitmap higher > than the existing mechanism? Do we need to look at starting with an > existing mechanism and then switching over to new mechanism? Is the TID > bitmap always a win for large numbers of rows? >
Thanks for looking at this. It works for all kinds of trigger events, and is intended as a complete drop-in replacement for the after triggers queue. I admit that I haven't yet done very much performance testing. As it stands, there does appear to be a small performance penalty associated with the bitmaps, but I need to do more testing to be more specific about that. I had thought that, for relatively small numbers of rows, I could use something like a small list of CTID arrays of increasing size, and then switch over to the new mechanism when this becomes too large. But first, I wanted to get feedback on whether this TID bitmap approach is actually valid for general trigger operation. > The technique relies on these assumptions > * Trigger functions are idempotent I don't understand what you're saying here. It should execute the triggers in exactly the same way as the current code (but possibly in a different order). Idempotentence isn't required. > * Trigger execution order is not important (in terms of rows) It is true that the order in which the rows are processed will change. As far as I can tell from the spec, there is nothing to say that the rows for a given statement should be processed in any particular order. I guess that I'm looking for feedback from people on this list as to whether that will be a problem for existing apps. > * Multiple trigger execution order is not important This patch does not change the order of execution in the case where there are multiple triggers (at least not for regular non-constraint triggers). They should still be fired in name order, for each row. All such triggers share a single TID bitmap, and are processed together. This is in line with the spec. Deferrable constraint triggers are a different matter, and these will be fired in a different order (each set of triggers for a given constraint will be fired together, rather than being interleaved). This is not covered by the spec, but if they are genuinely being used to enforce constraints, the order shouldn't matter. > > All of those seem false in the general case. What will you do? At this point I'm looking for more feedback as to whether any of this is a show-stopper, before I expend more effort on this patch. - Dean -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
