Re: Listen / Notify - what to do when the queue is full

On Wed, 2010-02-03 at 00:10 +0100, Joachim Wieland wrote:
> I admit that it was not clear what I meant. The comment should only
> address LISTEN / NOTIFY on the standby server. Do you see any problems
> allowing it?

The original comment was a part of the NotifyStmt case, and I don't
think we can support NOTIFY issued on a standby system -- surely there's
no way for the standby to communicate the notification to the master.
Anyway, this is getting a little sidetracked; I don't think we need to
worry about HS right now.

> I was wondering if we should have a hard limit on the maximal number
> of notifications per transaction. You can now easily fill up your
> backend's memory with notifications. However we had the same problem
> with the old implementation already and nobody complained. The
> difference is just that now you can fill it up a lot faster because
> you can send a large payload.

I don't see a need for that. The only way that is likely to happen is
with triggers, and there are other ways to fill up the memory using
triggers. Even if the option is there, all we can do is abort.

> The second doubt I had is about the truncation behavior of slru. ISTM
> that it doesn't truncate at the end of the page range once the head
> pointer has already wrapped around.
> There is the following comment in slru.c describing this fact:
>
> /*
> * While we are holding the lock, make an important safety check: the
> * planned cutoff point must be <= the current endpoint page. Otherwise we
> * have already wrapped around, and proceeding with the truncation would
> * risk removing the current segment.
> */
>
> I wanted to check if we can do anything about it and if we need to do
> anything at all...

I'll have to take a look in more detail.

> Now the question is, should we forbid NOTIFY for 2PC
> altogether only because in the unlikely event of a full queue we
> cannot guarantee that we can commit the transaction?

I believe that was the consensus in the thread. The people who use 2PC
are the people that want the most rock-solid guarantees. I don't like
forbidding feature combinations, but I don't see a good alternative.

> One solution is to treat a 2PC transaction like a backend with its own
> pointer to the queue. As long as the prepared transaction is not
> committed, its pointer does not move and so we don't move forward the
> global tail pointer. Here the NOTIFYs sent by the 2PC transaction are
> already in the queue and the transaction can always commit. The
> drawback of this idea is that if you forget to commit the prepared
> transaction and leave it around uncommitted, your queue will fill up
> inevitably because you do not truncate anymore...

There's also a problem if the power goes out, you restart, and then the
queue fills up before you COMMIT PREPARED.

> > * There's a bug where an UNLISTEN can abort, and yet you still miss
> > the notification.
> > [...]
> > The notification is missed. It's fixed easily enough by doing the
> > UNLISTEN step in AtCommit_NotifyAfterCommit.
>
> Thanks, very well spotted... Actually the same is true for LISTEN... I
> have reworked the patch to do the changes to listenChannels only in
> the post-commit functions.

I'm worried that this creates the opposite problem: that a LISTEN
transaction might commit before a NOTIFY transaction, and yet miss the
notification.

It seems safest to me to add a backend (LISTEN) to the list before
commit, and remove a backend (UNLISTEN) after commit. That way we are
sure to only receive spurious notifications, and can't miss any.

> I have also included Arnaud Betremieux's send_notify function. Should
> we really call the function send_notify? What about
> pg_send_notification or just pg_notify?

Agreed: pg_notify sounds good to me.

> Is [void] the preferred return type?

I can't think of anything else that it might return. NOTIFY doesn't
return much ;)

Regards,
Jeff Davis