Re: Expanding HOT updates for expression and partial indexes

Hello all,

Attached is v41, now ten patches, rebased on master (0f3b42ce806). v39
fails check_subattr on cfbot (a collation-dependent sort in the test,
below); that's fixed, along with a handful of smaller problems review
found in 0001-0008. The two items v39 left open are now new patches
0009 and 0010.

Let's a lot to review, so let's focus up front on a few key things and
save the benchmark results for later:

* 0009 changes unique_key_recheck(), the deferred-constraint trigger, for
exclusion constraints. It used table_fetch_tid(), which treats a
heap-only tuple at the TID as the end of a broken chain. With 0009, a
recheck can be queued for a HOT version, and that recheck passed without
checking anything, so a deferred EXCLUDE could commit a conflict. For
exclusion constraints it now follows the row to its live version under
SnapshotSelf and checks that version's values. Unique constraints keep
the old path. I'd like eyes on whether "the live version's values" is
the right thing to check in every case. The tests cover the row being
updated again or deleted before commit, the other row moving away, and
SET CONSTRAINTS IMMEDIATE.

* 0009 runs check_exclusion_or_unique_constraint() for a tuple that got
no new index entry: the old entries reach it through the HOT chain. A
concurrent inserter finds the in-progress tuple through those entries
and waits for it, and our check finds the inserter's entry; the new
isolation spec covers both orders. The argument is the one
unique_key_recheck() already relies on for a live HOT child, but it is
new for the immediate check.

* 0010 keeps pointers to the new row's index datums, which live in the
per-tuple context, in the ResultRelInfo until index insertion. They're
reset for each row, an EvalPlanQual retry decides again, and builds with
assertions recompute every reused value and compare. Getting this wrong
would put wrong entries in an index, so the lifetime is the thing to
check.

* An expensive index expression whose value changes is still evaluated
once more than master does: once for the old row, which deciding needs.
0010 removed the second extra evaluation. That is the largest
regression (+33-44% in the tests below). The other is smaller and
broader: whole-table UPDATEs the series can't make HOT run 3-4%
slower. Single-row UPDATEs show no cost.

Changes to 0001-0008 since v39 (no design change, no change in which
updates are HOT):

0001 hot_updates.sql: autovacuum off on the table whose HOT chain the
test prints, so it can't be pruned first; print_hot_chain() no
longer takes a chain's last tuple, which points at itself, as its
own predecessor; the test drops pageinspect at the end, leaving the
suite as oldextversions left it.

0002 simple_heap_update() leaked the modified-attributes Bitmapset on
every catalog update; fixed. heap_update() no longer fetches the
whole indexed-column bitmap just to intersect it with the replica
identity's. Its new parameters are documented and lose their
top-level const. Unrelated churn is gone: an include removed from
heapam_handler.c is back, an unneeded one added to
execReplication.c is dropped, and a reworded comment is restored.

simple_heap_update()'s comment no longer claims the executor
compares fewer columns than it does.

0003 ExecUpdateModifiedIdxAttrs() requires the EState; every caller
passes one, so the NULL case was dead.

0004 check_subattr sorted its results in the database collation; its
expected output came from glibc's en_US.UTF-8, which puts
"array-a-neg" after "array-a9-extend", while C, C.UTF-8, macOS and
Windows put it first. That is the cfbot failure on every platform
for v39. It now sorts with COLLATE "C", and runs at fillfactor 10 so
no count depends on page packing. The proof runs in a scratch
memory context and keeps only its bitmaps. An answer from a support
function with a null or unknown path element is treated as no answer
instead of crashing paths_may_overlap(), which now handles each pair
of element kinds explicitly. SUBATTR_ELEM_ANY, which nothing
produced, is gone. The two walkers that recurse on their own call
check_stack_depth(). The supportnodes.h contract now says that two
keys that both read as integers overlap. misc_functions also checks
HOT counts and pins an index scan, so a build without assertions
catches a broken proof. ExecInitUpdateUntouched() asserts that it
pairs SET columns only with non-junk target entries, the invariant
ExecBuildUpdateProjection() checks.

0005 The comparedatums test of key columns couldn't fail (a plain btree
on the same column blocked HOT anyway); it now shows the update HOT
first and not HOT once the unique index exists.

0006-0008 btcomparedatums()'s comment is corrected; GIN's entry loop
gets CHECK_FOR_INTERRUPTS(), as ginHeapTupleInsert() has; hash test
comments moved to the statements they describe; one check_btree
query sorts with COLLATE "C".

New:

0009 Keep HOT for exclusion constraints, and check them after the
update. 0005 never consulted amcomparedatums for an exclusion
constraint's index, because equal index entries need not mean equal
conflicts (hashint8(1) = hashint8(2^32)). Now, when the AM says the
entries stay the same although a value changed, the update may be
HOT, and ExecCheckHotExclusion() then checks the constraint for the
new tuple with its new values, with the wait mode and deferred
handling ExecInsertIndexTuples() uses (the two now share that
code). UPDATE, MERGE, logical replication apply and REPACK all do
this. The unique_key_recheck() fix is described above.

This matters most for things like the open source DocumentDB extension,
whose unique indexes are exclusion constraints over the whole document (a
single column of their opaque BSON type): a collection with even just one
unique exclusion constraint becomes a blocker for all HOT updates on that
table. Tests: hash_index (immediate and deferred), a new isolation spec
(hot-exclusion) for a concurrent inserter before and after the HOT
update, and logical replication apply in 032_subscribe_use_index.

0010 Reuse the new row's index datums, as Matthias suggested a very long time
ago. When 0003 has evaluated an index's predicate and expressions for
the new row and the update then needs index entries anyway,
ExecInsertIndexTuples() (and ExecCheckHotExclusion()) use those values
instead of evaluating them again. A regression test can't count
evaluations to show the reuse: in assert builds the cross-check evaluates
again, so the counts come out the same. The amcheck test checks
correctness instead, and the benchmarks below show the saving.

Performance, master (0f3b42ce806) against v41, release builds on a
c7i.8xlarge, three rounds interleaved, best run of each:

Updates the series makes HOT (whole-table UPDATE, ms):
master v41 HOT%
jsonb, expression indexes, other key 3608 777 78
partial index, row stays outside 885 483 80
btree, write back same large value 56974 934 79
GIN array, elements reordered 701 269 80
GIN to_tsvector, words reordered 1448 1051 79
hash, numeric display scale changed 1536 550 79
hash EXCLUDE, display scale changed 1726 931 98
toasted jsonb, 4 expr/partial indexes 1623 1491 79

HOT% is the share over five passes, with a VACUUM before each; the
rest don't fit on the page. An UPDATE of an unindexed column gets the
same 79% on master at this fillfactor.

Costs (ms or tps):
master v41
md5(2 kB text) index, value changes 791 1141 +44%
to_tsvector GIN index, value changes 1647 2185 +33%
jsonb expression index, value changes 4008 4182 +4%
md5 index, value same (HOT in v41) 760 812 +7%
indexed column changes (control) 2527 2615 +3%
hash EXCLUDE, value changes 1637 1690 +3%
no expression indexes (control) 383 397 +4%
catalog DDL loop (0002), tps 6663 6505 -2%

The first two rows are the extra evaluation of the old row. Before
0010 they were +80% and +58%. Where the md5 value stays the same, the
update is HOT, but one md5() of 2 kB costs more than the btree
insertion it saves. The two control rows and the EXCLUDE row are
3-4% slower in every round, as the controls were for v39 (+2.6%). I
haven't yet pinned down where those few percent go; for the plain
table no indexed column changes, so it is the per-row bookkeeping
rather than any evaluation. That is next on my list. The catalog DDL
loop moves 2-3% either way between rounds (v41 ahead in one of three).

Single-row prepared UPDATEs (pgbench, 20s, median of 9):
master v41
jsonb doc, expr indexes, c=1 36482 37555 +3%
jsonb doc, expr indexes, c=16 462920 472347 +2%
plain table, c=1 36885 37310 +1%
plain table, c=16 478949 484231 +1%

Index size after the five passes, where the updates become HOT: GIN
array 19072 kB on master, 16480 kB with v41; hash 58912 kB, 46848 kB.

gcov on 0001-0008's added lines over the full suite: 92% of lines, 76%
of branches. Just a reminder, this set of patches overlaps with the
series about selective index updates I call "Tepid" [1] and the impact
of this patch would be much higher were both to land.

best.

-greg

[1] https://commitfest.postgresql.org/patch/6968/