On Fri, 2009-11-20 at 10:35 +0100, Joachim Wieland wrote:
> On Thu, Nov 19, 2009 at 11:04 PM, Joachim Wieland <joe@mcknight.de> wrote:
> > Given your example, what I am proposing now is to stop reading from
> > the queue once we see a not-yet-committed notification but once the
> > queue is full, read the uncommitted notifications, effectively copying
> > them over into the backend's own memory... Once the transaction
> > commits and sends a signal, we can process, send and discard the
> > previously copied notifications. In the above example, at some point
> > one, two or all three backends would see that the queue is full and
> > everybody would read the uncommitted notifications of the other one,
> > copy them into the own memory and space will be freed in the queue.
>
> Attached is the patch that implements the described modifications.
This is a first-pass review.
Comments:
* Why don't we read all notifications into backend-local memory at
every opportunity? It looks like sometimes it's only reading the
committed ones, and I don't see the advantage of leaving it in the SLRU.
Code comments:
* I see a compiler warning:
ipci.c: In function ‘CreateSharedMemoryAndSemaphores’:
ipci.c:222: warning: implicit declaration of function ‘AsyncShmemInit’* sanity_check test fails, needs updating* guc
testfails, needs updating* no docs
The overall design looks pretty good to me. From my very brief pass over
the code, it looks like:* When the queue is full, the transaction waits until there is room
before it's committed. This may have an effect on 2PC, but the consensus
seemed to be that we could restrict the combination of 2PC + NOTIFY.
This also introduces the possibility of starvation, I suppose, but that
seems remote.* When the queue is full, the inserter tries to signal the listening
backends, and tries to make room in the queue.* Backends read the notifications when signaled, or when inserting (in
case the inserting backend is also the one preventing the queue from
shrinking).
I haven't looked at everything yet, but this seems like it's in
reasonable shape from a high level. Joachim, can you clean the patch up,
include docs, and fix the tests? If so, I'll do a full review.
Regards,Jeff Davis
