Re: [HACKERS] Think we need major revisions in async.c... - Mailing list pgsql-hackers
From | Tom Lane |
---|---|
Subject | Re: [HACKERS] Think we need major revisions in async.c... |
Date | |
Msg-id | 20822.907010106@sss.pgh.pa.us Whole thread Raw |
In response to | Re: [HACKERS] Think we need major revisions in async.c... (Massimo Dal Zotto <dz@cs.unitn.it>) |
List | pgsql-hackers |
Massimo Dal Zotto <dz@cs.unitn.it> writes: >> 1. The link between inbound-notify processing and transaction >> start/commit should be removed. Having inbound-notify execution called >> from transaction commit is too complicated because that notify execution >> itself may have to induce a transaction start/commit. Instead, inbound >> notify execution ought to be triggered from the PostgresMain main loop. > The reason why it is done at commit is that it is the best place where we > can know if the transaction completed or not. If we move the notify code > in the main loop we must also keep some flag which tell us if and when the > notify can be done. No, you're confusing outbound and inbound notify. Outbound notify --- ie, actually doing something about a NOTIFY command issued by the user --- should indeed be done from transaction commit. If the transaction is aborted, we want the NOTIFY to never take effect, just like all the other side-effects of the transaction. What I proposed for that was that we could simplify matters by doing both the signal-sending and pg_listener-table-updating work at the same time during transaction commit, rather than changing the table and issuing the signals in separate passes. Inbound notify, ie, receiving messages from other backends, is a different story. There is no question of not accepting the signals; they've already been committed by some other backend. Now for implementation reasons we need to hold off responding to an inbound notify until we are outside of any transaction. (Otherwise, our attempt to mark the notify accepted by changing pg_listener could be undone by a later transaction abort; not what we want.) But that doesn't mean the response processing has to be called from xact.c. Better to do it at the main loop, with a check to see if we are inside a transaction block or not. Otherwise we have to depend on stdio to be re-entrant. Also, xact.c is already tricky enough without having to worry about the exact order of operations in straightline code. I spent some time on Saturday trying to see if the variables the signal handler was looking at were being changed before the transaction was quite complete --- and I'm still not sure whether it was really safe, given the potential for mutual recursion between async.c and xact.c in the old code. Writing code that interacts correctly with an ISR is tricky stuff, so you really want to do it in a place that doesn't have much of anything else to worry about. > This is the standard behaviour of the notify code. If you set the flag > notifyunlock in .../data/pg_options the write lock is released immediately > after updating the table, but I suspect this is the reason of the > duplicate oids. For this reason I left the flag disabled by default, and > maybe this explains why you don't get duplicate oids in 6.4, but I have > enabled it in a production environment without problems at all. Neither NotifyUnlock nor NotifyHack are necessary in the rewritten code... >> This is a waste of cycles, but right now there is no >> choice because you cannot tell whether the NOTIFY you get is due >> (solely) to your own NOTIFY or (perhaps in part) to someone else's. > What you propose should be some sort of message queuing between backends > in which each message is notified separately to the other backend In this > case it would also be useful to pass some arbitrary text to the listening > backend. Not really. For the applications I'm interested in, there are only two interesting cases: a NOTIFY from some other client vs. one of your own NOTIFYs bouncing back at you. With the code as I now have it, you can differentiate those cases. You're right that a more general message-passing facility might be useful. On the other hand, one can be built by users using an ordinary table and NOTIFY to wake up sleeping respondents. So I'm not sure we need to put more stuff in the backend for that. I have been testing this new code and it seems to work fine, except for the question of interlocks against "select * from pg_listener" --- and if that's a bug, it's one that was there before anyway. So I'm ready to check it in. (Marc, are you there?) regards, tom lane
pgsql-hackers by date: