On Tue, Feb 16, 2010 at 11:41 PM, Tom Lane <t...@sss.pgh.pa.us> wrote: > Joachim Wieland <j...@mcknight.de> writes: >> [ listen/notify patch ] > > I found a number of implementation problems having to do with wraparound > behavior and error recovery. I think they're all fixed, but any > remaining bugs are probably my fault not yours.
First, thanks for the rework you have done and thanks for applying this. While I can see a lot of improvements over my version, I think the logic in asyncQueueProcessPageEntries() needs to be reordered: + static bool + asyncQueueProcessPageEntries(QueuePosition *current, + QueuePosition stop, + char *page_buffer) [...] + do + { [...] + /* + * Advance *current over this message, possibly to the next page. + * As noted in the comments for asyncQueueReadAllNotifications, we + * must do this before possibly failing while processing the message. + */ + reachedEndOfPage = asyncQueueAdvance(current, qe->length); [...] + if (TransactionIdDidCommit(qe->xid)) [...] + else if (TransactionIdDidAbort(qe->xid)) [...] + else + { + /* + * The transaction has neither committed nor aborted so far, + * so we can't process its message yet. Break out of the loop. + */ + reachedStop = true; + break; In the beginning you are advancing *current but later on you could find out that the transaction is still running. As the position in the queue has already advanced you would miss one notification here because you'd restart directly behind this notification in the queue... Joachim -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers