Re: On-demand running query plans using auto_explain and signals - Mailing list pgsql-hackers

On 09/14/2015 01:15 PM, Shulgin, Oleksandr wrote:

On Mon, Sep 14, 2015 at 11:53 AM, Tomas Vondra
<tomas.vondra@2ndquadrant.com <mailto:tomas.vondra@2ndquadrant.com>> wrote:


    On 09/14/2015 10:23 AM, Shulgin, Oleksandr wrote:

        On Sat, Sep 12, 2015 at 11:50 AM, Tomas Vondra
        <tomas.vondra@2ndquadrant.com
        <mailto:tomas.vondra@2ndquadrant.com>
        <mailto:tomas.vondra@2ndquadrant.com

        <mailto:tomas.vondra@2ndquadrant.com>>> wrote:

    ...

             - Attempts to get plan for simple insert queries like this

                    INSERT INTO x SELECT * FROM x;

                end with a segfault, because ActivePortal->queryDesc is
        0x0 for this
                query. Needs investigation.


        Yes, I've hit the same problem after submitting the latest
        version of
        the patch.  For now I've just added a check for queryDesc being not
        NULL, but I guess the top of the current_query_stack might contains
        something useful.  Something I need to try.


    Well, the thing is we're able to do EXPLAIN on those queries, and
    IIRC auto_explain can log them too. So perhaps look into the hooks
    where they take the queryDesc in those cases - it has to be
    available somewhere.


Yes, this seems to work.  I was able to successfully query "create table
as" and even "explain analyze" for the plans.  In both cases
ActivePortal doesn't have a valid queryDesc, but the executor hook
captures one.

             - The lockless approach seems fine to me, although I think
        the fear
                of performance issues is a bit moot (I don't think we
        expect large
                number of processes calling pg_cmdstatus at the same
        time). But
                it's not significantly more complex, so why not.


        I believe the main benefit of the less-locking approach is that if
        something goes wrong when two backends tried to communicate it
        doesn't
        prevent the rest of them from operating, because there is no
        shared (and
        thus locked) communication channel.


    Sure, but I think it really deserves a bit more detailed explanation
    of the protocol, and discussion of the expected behavior for some
    basic failure types.

    For example - what happens when a backend requests info, but dies
    before receiving it, and the backed is reused for another
    connection? Doesn't this introduce a race condition? Perhaps not,
    I'm just asking.


Now explained better in code comments.

The worst thing that could happen in this case I believe is that the
requesting backend will not receive any response from the second use of
its slot due to the slot being marked as processed by the backend that
was asked first:

A:
slot->is_processed = false;
slot->is_valid = true;
/* signals backend B */;
shm_mq_read(); /* blocks */

B: /* finds the slot and prepares the response */

A: /* decides to bail out */
InvalidateCmdStatusSlot();
shm_mq_detach();
/* exits pg_cmdstatus() */

/* pg_cmdstatus() is called again */
/* initializes the slot and signals some backend */
shm_mq_read(); /* blocks */

B: /* completes preparing the response */
slot->is_processed = true;
shm_mq_send() => SHM_MQ_DETACHED
slot->is_valid = false;
/* gives up with this slot */