Re: pg_background (and more parallelism infrastructure patches)

On Tue, Sep 9, 2014 at 6:03 PM, Petr Jelinek <petr@2ndquadrant.com> wrote:
>> - The remaining functions are pq_init(), pq_comm_reset(), pq_flush(),
>> pq_flush_if_writable(), pq_is_send_pending(), pq_putmessage(),
>> pq_putmessage_noblock(), pq_startcopyout(), and pq_endcopyout().
>> These are the ones that I think are potentially interesting.
>>
>> I didn't choose to provide hooks for all of these in the submitted
>> patch because they're not all needed for I want to do here:
>> pq_startcopyout() and pq_endcopyout() are only needed for V2 protocol
>> support, which did not interest me (nor did COPY, really);
>> pq_putmessage_noblock(), pq_flush_if_writable(), and
>> pq_is_send_pending() are only used for the walsender protocol, which
>> doesn't seem useful to redirect to a non-socket; and I just didn't
>> happen to have any use for pq_init() or pq_comm_reset().  Hence what I
>> ended up with.
>>
>> But, I could revisit that.  Suppose I provide a structure with 10
>> function pointers for all ten of those functions, or maybe 9 since
>> pq_init() is called so early that it's not likely we'll have control
>> to put the hooks in place before that point, and anyway whatever code
>> installs the hooks can do its own initialization then.  Then make a
>> global variable like pqSendMethods and #define pq_comm_reset() to be
>> pqSendMethods->comm_reset(), pflush() to be pqSendMethods->flush(),
>> and so on for all 9 or 10 methods.  Then the pqmq code could just
>> change pqSendMethods to point to its own method structure instead of
>> the default one.  Would that address the concern this concern?  It's
>> more than 20 lines of code, but it's not TOO bad.
>>
>
> Yes, although my issue with the hooks was not that you only provided them
> for 2 functions, but the fact that it had no structure and the
> implementation was "if hook set do this, else do that" which I don't see
> like a good way of doing it.

We've done it that way in a bunch of other places, like ExecutorRun().
An advantage of this approach (I think) is that jumping to a fixed
address is faster than jumping through a function pointer - so with
the approach I've taken here, the common case where we're talking to
the client incurs only the overhead of a null-test, and the larger
overhead of the function pointer jump is incurred only when the hook
is in use.  Maybe that's not enough of a difference to matter to
anything, but I think the contention that I've invented some novel
kind of interface here doesn't hold up to scrutiny.  We have lots of
hooks that work just like what I did here.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company