On Fri, Jun 25, 2021 at 4:44 PM Peter Geoghegan <pg@bowt.ie> wrote:
>
> On Fri, Jun 25, 2021 at 1:43 AM Simon Riggs
> <simon.riggs@enterprisedb.com> wrote:
> > Seems a little bizarre to have _bt_check_unique() call back into the
> > heap block we literally just unpinned.
>
> I suppose it is a little bizarre.
>
> > This is another case of the UPDATE scan and later heap/index
> > insertions not working together very well.
> > This makes this case even harder to solve:
> > https://www.postgresql.org/message-id/CA%2BU5nMKzsjwcpSoqLsfqYQRwW6udPtgBdqXz34fUwaVfgXKWhA%40mail.gmail.com
>
> I wasn't aware of that thread, but I suspected that something like
> that was going on in some cases myself.
>
> > If an UPDATE interferes with its own ability to kill_prior_tuple(),
> > then we should fix it, not allow pointless work to be performed
> > somewhere else instead just because it has some beneficial side
> > effect.
>
> Definitely true. But the fact is that this is where we are today, and
> that complicates this business with bypassing _bt_check_unique().
>
> > If an UPDATE scans via a index and remembers the block in
> > so->currPos.currPage then we could use that to optimize the
> > re-insertion by starting the insertion scan at that block (since we
> > know the live unique key is either there or somewhere to the right).
> > By connecting those together, we would then be able to know that the
> > change in LSN was caused by ourself and allow the items to be killed
> > correctly at that time.
> >
> > Do you think there is benefit in having PK UPDATEs as a special plan
> > that links these things more closely together?
>
> I think that it might be worth hinting to the index scan that it is
> feeding a ModifyTable node, and that it should not drop its pin per
> the optimization added to avoid blocking VACUUM (in commit
> 2ed5b87f96d). We can just not do that if for whatever reason we don't
> think it's worth it - the really important cases for that optimization
> involve cursors, things like that.
>
> It's not like the code that deals with this (that notices LSN change)
> cannot just recheck by going to the heap. The chances of it really
> being VACUUM are generally extremely low.
>
> OTOH I wonder if the whole idea of holding a pin on a leaf page to
> block VACUUM is one that should be removed entirely.
Definitely some good ideas here.
I'm out of time to do anything for this CF, so I've moved this back to later CF.
I'm planning to work on this more, but I won't try to fold in all of
your ideas above. Not cos they are bad ones, just there is enough room
for 2-4 related patches here.
--
Simon Riggs http://www.EnterpriseDB.com/
