Listen / Notify - what to do when the queue is full
in [PgSql]
|
From: Tom Lane on 16 Feb 2010 17:41 Joachim Wieland <joe(a)mcknight.de> writes: > [ listen/notify patch ] Applied after rather a lot of hacking. Aside from the issues previously raised, I changed the NOTIFY syntax to include a comma between channel name and payload. The submitted syntax with no comma looked odd to me, and it would have been a real nightmare to extend if we ever decide we want to support expressions in NOTIFY. 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. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Joachim Wieland on 17 Feb 2010 05:16 On Tue, Feb 16, 2010 at 11:41 PM, Tom Lane <tgl(a)sss.pgh.pa.us> wrote: > Joachim Wieland <joe(a)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(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Tom Lane on 17 Feb 2010 10:33 Joachim Wieland <joe(a)mcknight.de> writes: > While I can see a lot of improvements over my version, I think the > logic in asyncQueueProcessPageEntries() needs to be reordered: Hmmm ... I was intending to cover the case of a failure in TransactionIdDidCommit too, but I can see it will take a bit more thought. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Simon Riggs on 18 Feb 2010 12:58 On Mon, 2010-02-15 at 15:00 -0500, Tom Lane wrote: > Joachim Wieland <joe(a)mcknight.de> writes: > > We could probably fake this on the Hot Standby in the following way: > > > We introduce a commit record for every notifying transaction and write > > it into the queue itself. So right before writing anything else, we > > write an entry which informs readers that the following records are > > not yet committed. Then we write the actual notifications and commit. > > In post-commit we return back to the commit record and flip its > > status. > > This doesn't seem likely to work --- it essentially makes commit non > atomic. There has to be one and only one authoritative reference as > to whether transaction X committed. I thought a bit more about this and don't really understand why we need an xid at all. When we discussed this before the role of a NOTIFY was to remind us to refresh a cache, not as a way of delivering a transactional payload. If the cache refresh use case is still the objective why does it matter whether we commit or not when we issue a NOTIFY? Surely, the rare case where we actually abort right at the end of the transaction will just cause an unnecessary cache refresh. > I think that having HS slave sessions issue notifies is a fairly silly > idea anyway. They can't write the database, so exactly what condition > are they going to be notifying others about? Agreed > What *would* be useful is for HS slaves to be able to listen for notify > messages issued by writing sessions on the master. This patch gets rid > of the need for LISTEN to change on-disk state, so in principle we can > do it. The only bit we seem to lack is WAL transmission of the messages > (plus of course synchronization in case a slave session is too slow > about picking up messages). Definitely a 9.1 project at this point > though. OK -- Simon Riggs www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Josh Berkus on 18 Feb 2010 13:11
On 2/18/10 9:58 AM, Simon Riggs wrote: > I thought a bit more about this and don't really understand why we need > an xid at all. When we discussed this before the role of a NOTIFY was to > remind us to refresh a cache, not as a way of delivering a transactional > payload. If the cache refresh use case is still the objective why does > it matter whether we commit or not when we issue a NOTIFY? Surely, the > rare case where we actually abort right at the end of the transaction > will just cause an unnecessary cache refresh. Actually, even for that use, it doesn't wash to have notifies being sent for transactions which have not yet committed. Think of cases (and I have two applications) where data is being pushed into an external non-transactional cache or queue (like Memcached, Redis or ApacheMQ) from PostgreSQL on the backend. If the transaction fails, it's important that the data not get pushed. I guess I'm not following why HS would be different from a single server in this regard, though? Mind you, this is all 9.1 discussion, no? --Josh Berkus -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers |