On 11/21/19 8:03 PM, Tom Lane wrote:
Mark Dilger <hornschnor...@gmail.com> writes:
I have winnowed down the test a bit further.  The attached
smaller patch still triggers the same assertion as the prior
patch did.

FWIW, I can reproduce the assertion failure with your first test,
but not with this simplified one.

Thanks for checking!

I also confirm that it only happens in HEAD, not v12.  I've not
actually bisected, but a look at the git history for predicate.c
sure makes it look like db2687d1f ("Optimize PredicateLockTuple")
must be to blame.

`git bisect` shows the problem occurs earlier than that, and by
chance the first bad commit was one of yours.  I'm not surprised
that your commit was regarding LISTEN/NOTIFY, as the error is
always triggered with a LISTEN statement.  (I've now hit this
many times in many tests of multiple SQL statements, and the
last statement before the error is always a LISTEN.)

I'm cc'ing Martijn because he is mentioned in that commit.


51004c7172b5c35afac050f4d5849839a06e8d3b is the first bad commit
commit 51004c7172b5c35afac050f4d5849839a06e8d3b
Author: Tom Lane <t...@sss.pgh.pa.us>
Date:   Sun Sep 22 11:46:29 2019 -0400

    Make some efficiency improvements in LISTEN/NOTIFY.

    Move the responsibility for advancing the NOTIFY queue tail pointer
    from the listener(s) to the notification sender, and only have the
    sender do it once every few queue pages, rather than after every batch
    of notifications as at present.  This reduces the number of times we
    execute asyncQueueAdvanceTail, and reduces contention when there are
    multiple listeners (since that function requires exclusive lock).
This change relies on the observation that we don't really need the tail
    pointer to be exactly up-to-date.  It's certainly not necessary to
    attempt to release disk space more often than once per SLRU segment.
    The only other usage of the tail pointer is that an incoming listener,
    if it's the only listener in its database, will need to scan the queue
    forward from the tail; but that's surely a less performance-critical
    path than routine sending and receiving of notifies.  We compromise by
    advancing the tail pointer after every 4 pages of output, so that it
    shouldn't get more than a few pages behind.

    Also, when sending signals to other backends after adding notify
    message(s) to the queue, recognize that only backends in our own
    database are going to care about those messages, so only such
    backends really need to be awakened promptly.  Backends in other
    databases should get kicked if they're well behind on reading the
    queue, else they'll hold back the global tail pointer; but wakening
    them for every single message is pointless.  This change can
    substantially reduce signal traffic if listeners are spread among
    many databases.  It won't help for the common case of only a single
    active database, but the extra check costs very little.

    Martijn van Oosterhout, with some adjustments by me

Discussion: https://postgr.es/m/cadwg95vtrbfddrx1jdt1_9nhofw48kaetev6f_ltdqafvps...@mail.gmail.com Discussion: https://postgr.es/m/cadwg95ufj8rlm52er80jnhrstbb_aqpp1anhs8xqrgbqlru...@mail.gmail.com


--
Mark Dilger


Reply via email to