Horton

Thread

Commits

  1. Go back to considering HOT on pages marked full.

  2. Avoid improbable PANIC during heap_update.

  3. Remove tupgone special case from vacuumlazy.c.

  4. Simplify check of modified attributes in heap_update

  1. Why not try for a HOT update, even when PageIsFull()?

    Peter Geoghegan <pg@bowt.ie> — 2021-11-18T03:18:45Z

    Commit 2fd8685e7f simplified the checking of modified attributes that
takes place within heap_update(). This included a micro-optimization
that affects pages marked PageIsFull(): when the target page is marked
with PD_PAGE_FULL (which must have been set by a previous heap_update
call), don't even try to use HOT -- assume that we have no chance in
order to save a few cycles on determining HOT safety.

I doubt that this micro-optimization actually pays for itself, though.
Plus heap_update() is very complicated; do we really need to keep this
special case? The benefit is that we avoid work that we do ~99% of the
time anyway.

Attached patch removes the micro-optimization entirely. This isn't
just refactoring work -- at least to me. I'm also concerned that this
test unnecessarily prevents HOT updates with certain workloads.

There is a reasonable chance that the last updater couldn't fit a new
version on the same heap page (and so marked the page PD_PAGE_FULL) at
a point where the page didn't quite have enough free space for *their*
new tuple, while *almost* having enough space. And so it's worth being
open to the possibility that our own heap_update() call has a smaller
tuple than the first updater, perhaps only by chance (or perhaps
because the original updater couldn't use HOT specifically because
their new tuple was unusually large). Not all tables naturally have
equisized rows, obviously. And even when they do we should still
expect TOAST compression to create variation in the size of physical
heap tuples (some toastable attributes have more entropy than others,
making them less effective targets for compression, etc).

It's not just variability in the size of heap tuples. Comments
describing the micro-optimization claim that there is no chance of
cleanup happening concurrently, so that can't be a factor. But that's
really not true anymore. While it is still true that heap_update holds
a pin on the original page, blocking concurrent pruning (e.g., while
it waits for a tuple heavyweight lock), that in itself doesn't mean
that nobody else can free up space when heap_update() drops its buffer
lock -- things have changed. Commit 8523492d4e taught VACUUM to set
LP_DEAD line pointers to LP_UNUSED, while only holding an exclusive
lock (not a super-exclusive/cleanup lock) on the target heap
page/buffer. That's enough to allow concurrent processing by VACUUM to
go ahead (excluding pruning). And so PageGetHeapFreeSpace() can go
from indicating that the page has 0 space to more than enough space,
due only to concurrent activity by VACUUM (a pin won't prevent that
anymore). This is not especially unlikely with a small table.

I think that it's possible that Tom would have found it easier to
debug an issue that led to a PANIC inside heap_update() earlier this
year (see commit 34f581c39e). That bug was judged to be an old bug in
heap_update(), but we only started to see PANICs when the
aforementioned enhancement to VACUUM went in.

-- 
Peter Geoghegan

  2. Re: Why not try for a HOT update, even when PageIsFull()?

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2021-11-19T19:50:54Z

    On 2021-Nov-17, Peter Geoghegan wrote:

> Commit 2fd8685e7f simplified the checking of modified attributes that
> takes place within heap_update(). This included a micro-optimization
> that affects pages marked PageIsFull(): when the target page is marked
> with PD_PAGE_FULL (which must have been set by a previous heap_update
> call), don't even try to use HOT -- assume that we have no chance in
> order to save a few cycles on determining HOT safety.

Hmm, I don't have any memory of introducing this; and if you look at the
thread, you'll notice that it got there between the first patch I posted
and the second one, without any mention of the reason.  I probably got
that code from the WARM patch series at some point, thinking that it was
an obvious optimization; but I'm fairly certain that we didn't run any
tailored micro-benchmark to justify it.  Pavan may have something to say
about it, so I CC him.

I certainly do not object to removing it.

-- 
Álvaro Herrera           39°49'30"S 73°17'W  —  https://www.EnterpriseDB.com/
"Find a bug in a program, and fix it, and the program will work today.
Show the program how to find and fix a bug, and the program
will work forever" (Oliver Silfridge)

  3. Re: Why not try for a HOT update, even when PageIsFull()?

    Peter Geoghegan <pg@bowt.ie> — 2021-11-22T00:29:11Z

    On Fri, Nov 19, 2021 at 11:51 AM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> Hmm, I don't have any memory of introducing this; and if you look at the
> thread, you'll notice that it got there between the first patch I posted
> and the second one, without any mention of the reason.  I probably got
> that code from the WARM patch series at some point, thinking that it was
> an obvious optimization; but I'm fairly certain that we didn't run any
> tailored micro-benchmark to justify it.

I suspected that it was something like that. I agree that it's
unlikely that we'll be able to do another HOT update for as long as
the page has PD_PAGE_FULL set. But that's not saying much; it's also
unlikely that heap_update will find that PD_PAGE_FULL is set to begin
with. And, the chances of successfully applying HOT again are workload
dependent.

> I certainly do not object to removing it.

I'd like to do so soon. I'll wait a few more days, in case Pavan objects.

Thanks
-- 
Peter Geoghegan

  4. Re: Why not try for a HOT update, even when PageIsFull()?

    Peter Geoghegan <pg@bowt.ie> — 2021-11-26T18:59:25Z

    On Sun, Nov 21, 2021 at 4:29 PM Peter Geoghegan <pg@bowt.ie> wrote:
> On Fri, Nov 19, 2021 at 11:51 AM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> > I certainly do not object to removing it.
>
> I'd like to do so soon. I'll wait a few more days, in case Pavan objects.

Pushed just now.

Thanks
-- 
Peter Geoghegan