Re: Listen / Notify - what to do when the queue is full - Mailing list pgsql-hackers

Regarding the send_notify function, I have been working on it and have a
patch (attached) that applies on top of Joachim's.

It introduces a send_notify SQL function that calls the Async_Notify C
function.

It's pretty straightforward and will need to be refined to take into
account the encoding decisions that are made in Async_Notify, but it
should be a good starting point, and it doesn't introduce much in the
way of complexity.

On a side note, since that might be a discussion for another thread, it
would be nice and more DBA friendly, to have some kind of syntactic
sugar around it to be able to use
NOTIFY channel (SELECT col FROM table);
instead of
SELECT send_notify('channel', (SELECT col FROM table));

Best regards,
Arnaud Betremieux

On 19/01/2010 08:08, Jeff Davis wrote:
> Initial comments:
>
> * compiler warnings
>
> ipci.c: In function ‘CreateSharedMemoryAndSemaphores’:
> ipci.c:228: warning: implicit declaration of function ‘AsyncShmemInit’
>
> * 2PC
>
> Adds complexity, and I didn't see any clear, easy solution after
> reading the thread. I don't see this as a showstopper, so I'd leave
> this until later.
>
> * Hot Standby
>
> It would be nice to have NOTIFY work with HS, but I don't think that's
> going to happen this cycle. I don't think this is a showstopper,
> either.
>
> * ASCII?
>
> I tend to think that encoding should be handled properly here, or we
> should use BYTEA. My reasoning is that TEXT is a datatype, whereas ASCII
> is not, and I don't see a reason to invent it now. We might as well use
> real TEXT (with encoding support) or use BYTEA which works fine for
> ASCII anyway.
>
> * send_notify()
>
> Someone suggested a function like this, which sounds useful to me. Not
> a showstopper, but if it's not too difficult it might be worth
> including. (Incidentally, the function signature should match the type
> of the payload, which is unclear if we're going to use ASCII.)
>
> * AsyncCommitOrderLock
>
> This is a big red flag to me. The name by itself is scary. The fact
> that it is held across a series of fairly interesting operations is
> even scarier.
>
> As you say in the comments, it's acquired before recording the
> transaction commit in the clog, and released afterward. What you
> actually have is something like:
>
>      AtCommit_NotifyBeforeCommit();
>
>      HOLD_INTERRUPTS();
>
>      s->state = TRANS_COMMIT;
>
>      latestXid = RecordTransactionCommit(false);
>
>      TRACE_POSTGRESQL_TRANSACTION_COMMIT(MyProc->lxid);
>
>      ProcArrayEndTransaction(MyProc, latestXid);
>
>      CallXactCallbacks(XACT_EVENT_COMMIT);
>
>      ResourceOwnerRelease(TopTransactionResourceOwner,
>                           RESOURCE_RELEASE_BEFORE_LOCKS,
>                           true, true);
>
>      AtEOXact_Buffers(true);
>
>      AtEOXact_RelationCache(true);
>
>      AtEarlyCommit_Snapshot();
>
>      AtEOXact_Inval(true);
>
>      smgrDoPendingDeletes(true);
>
>      AtEOXact_MultiXact();
>
>      AtCommit_NotifyAfterCommit();
>
> That is a lot of stuff happening between the acquisition and
> release. There are two things particularly scary about this (to me):
>
>   * holding an LWLock while performing I/O (in smgrDoPendingDeletes())
>
>   * holding an LWLock while acquiring another LWLock (e.g.
>     ProcArrayEndTransaction())
>
> An LWLock-based deadlock is a hard deadlock -- not detected by the
> deadlock detector, and there's not much you can do even if it were --
> right in the transaction-completing code. That means that the whole
> system would be locked up. I'm not sure that such a deadlock condition
> exists, but it seems likely (if not, it's only because other areas of
> the code avoided this practice), and hard to prove that it's safe. And
> it's probably bad for performance to increase the length of time
> transactions are serialized, even if only for cases that involve
> LISTEN/NOTIFY.
>
> I believe this needs a re-think. What is the real purpose for
> AsyncCommitOrderLock, and can we acheive that another way? It seems
> that you're worried about a transaction that issues a LISTEN and
> committing not getting a notification from a NOTIFYing transaction
> that commits concurrently (and slightly after the LISTEN). But
> SignalBackends() is called after transaction commit, and should signal
> all backends who committed a LISTEN before that time, right?
>
> * The transaction IDs are used because Send_Notify() is called before
> the AsyncCommitOrderLock acquire, and so the backend could potentially
> be reading uncommitted notifications that are "about" to be committed
> (or aborted). Then, the queue is not read further until that transaction
> completes. That's not really commented effectively, and I suspect the
> process could be simpler. For instance, why can't the backend always
> read all of the data from the queue, notifying if the transaction is
> committed and saving to a local list otherwise (which would be checked
> on the next wakeup)?
>
> * Can you clarify the big comment at the top of async.c? It's a helpful
> overview, but it glosses over some of the touchy synchronization steps
> going on. I find myself trying to make a map of these steps myself.
>
> Regards,
>     Jeff Davis
>
>
>