Matt Newell <newe...@blur.com> writes:
> Patch attached works great.  I added the dboid to the QueueBackendStatus 
> struct but that might not be needed if there is an easy and fast way to get a 
> db oid from a backend pid.

Not particularly; I agree with adding it to this data structure.  One
reason is it makes the array stride a power of 2, so array accesses may be
a shade cheaper than before.

> I also skip the max pos calculation loop if QUEUE_HEAD is on the same page as 
> QUEUE_TAIL, with the thought that it's not desirable to increase the time 
> that 
> AsyncQueueLock is held if the queue is small and 
> asyncQueueReadAllNotifications will execute quickly.

Check.

There were a couple of minor bugs in this, but I fixed them and committed
it.

> I then noticed that we are taking the same lock twice in a row to read the 
> same values, first in Exec_ListenPreCommit then in 
> asyncQueueReadAllNotifications, and much of the time the latter will simply 
> return because pos will already be at QUEUE_HEAD.  I prepared a second patch
> that splits asyncQueueReadAllNotifications. Exec_ListPreCommit then only 
> calls 
> the worker version only when needed.  This avoids the duplicate lock.

I was unimpressed with this; the refactoring of asyncQueueReadAllNotifications
seemed messy and definitely was undocumented.  I think the main actual
value is to skip doing anything during Exec_ListenPreCommit when
max == head, so I extracted that part and included it.

                        regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to