Re: pg_background (and more parallelism infrastructure patches)

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:

- StreamServerPort, StreamConnection, StreamClose, and
TouchSocketFiles are intended to be called only from the postmaster,
to set up and tear down the listening socket and individual
connections. Presumably this is not what we care about here.
- pq_getbytes(), pq_getstring(), pq_getmessage(), pq_getbyte(),
pq_peekbyte(), and pq_getbyte_if_available() handle *reading* data
from the socket. Since you previously agreed that we didn't need to
build two-way communication on top of this, I would thank that would
mean that these don't need to be pluggable either. But maybe I'm
wrong.
- 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.

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