Re: pg_background (and more parallelism infrastructure patches)

On Wed, Sep 10, 2014 at 2:18 AM, Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Tue, Sep 9, 2014 at 1:18 PM, Petr Jelinek <petr@2ndquadrant.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.