Re: pg_background (and more parallelism infrastructure patches)

On 09/09/14 22:48, Robert Haas wrote:
> On Tue, Sep 9, 2014 at 1:18 PM, Petr Jelinek <petr(at)2ndquadrant(dot)com> wrote:
>>> I think that's completely wrong. As the patch series demonstrates,
>>> it's not limited to propagating ErrorResponse and NoticeResponse. It
>>> can also propagate NotifyResponse and RowDescription and DataRow and
>>> anything else that comes along. We are not just propagating errors;
>>> we are propagating all protocol messages of whatever type. So tying
>>> it to elog specifically is not right.
>>
>> Oh in that case, I think what Andres proposed is actually quite good. I know
>> the hook works fine it just seems like using somewhat hackish solution to
>> save 20 lines of code.
>
> If it's 20 lines of code, I'm probably fine to go that way. Let's see
> if we can figure out what those 20 lines look like.
>
> libpq.h exports 29 functions that do a variety of different things.
> Of those, 20 are in pqcomm.c and the others are in be-secure.c. I
> presume that pluggability for the latter group, if needed at all, is a
> separate project. The remaining ones break down like this:
>

Ugh

> - 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.

All I personally want is structure and extensibility so struct with just
the needed functions is good enough for me and I would be ok to leave
the other 8 functions just as a option for whomever needs to make them
overridable in the future.

--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services