Re: Expanding HOT updates for expression and partial indexes - Mailing list pgsql-hackers

Hello Jeff, hackers,

In v33 I've updated a test in triggers.sql to address differences across platforms identified by the cf-bot and rebased
thework.
 

I thought it might be prudent to add tests that validate all the corner cases of HOT that I could come up with, maybe
toomany (you tell me).  In addition, because code that impacts HOT is also involved in what is WAL logged for the
purposesof logical replication, I've added tests that try to explore the corners of that too.  The goal of these first
fewpatches is to NOT change the behavior of these things, but to only move the logic into the executor and out of heap
thenit makes sense to validate that explicitly.
 

At some point when I get back to $subjet I'll want to document how things changed.  The best way to do that is by
changingtests along with code.  So, that is "0001" in this v33 patch set.
 

I've also run longer performance tests which show minimal performance differences between master and patched.

Workload                60s     300s    600s
jsonb_write_batch       +14.8%  -7.0%   +0.1%
jsonb_write_single      +0.3%   +0.2%   -0.0%
license_write_single    +0.4%   +0.2%   -0.1%
gin_write_single        -0.3%   -0.2%   -0.4%
pgbench_simple-update   +3.2%   +6.2%   +0.9%
pgbench_tpcb-like       -1.1%   +0.8%   -2.0%

Changing tests isn't something I take lightly, I dug into this quite a bit.  I ran an analysis of ALL regression tests
comparingmaster vs patched after instrumenting the code (see below) so I could record HOT and replica identity
decisionsand record where the tuple landed on the page.
 

Patched code produced:
  simple_heap_update: 17,028 calls (72.5% - catalog updates, direct heap ops)
  heapam_tuple_update: 6,462 calls (27.5% - executor path via table AM)
  Total entry points: 23,490

This matched master's log line output for the same tests.

Replica identity decisions were identical, 342 unique patterns with 0 differences.

HOT eligibility was also identical, 398 unique patterns matched, again 0 differences.
  
The physical placement of tuples on pages was 99.991% identical, only 2 of 23,473 updates had different buffer
placement.

Across test runs there were a few differences noted for pg_sequence, target, and wslot.  Both master and patched agreed
onhot_allowed=1 (logic identical), but in some cases use_hot_update differed (buffer placement, newbuf =?= buffer).  To
methis reads as non-deterministic behavior, not a bug introduced in this patch.
 

At this point I'd say that v33 patch is functionally correct and performance neutral.  This set of changes isn't
exactlyexciting on the surface, but I feel that it opens the door to other changes that will be more
interesting/valuabledown the line.
 

Thank you for your time and interest.

best.

-greg


COMPARISON TESTING NOTES:
---------------------------------------------------------------------------------------
src/backend/access/heap/heapam.c
3514:    elog(LOG, "PATCHED heap_update (replica check): rel=%s otid=(%u,%u) rep_id_key_required=%d",
3515-         RelationGetRelationName(relation),
3516-         ItemPointerGetBlockNumber(otid),
3517-         ItemPointerGetOffsetNumber(otid),
3518-         rep_id_key_required);
3519-
--
4106:    elog(LOG, "PATCHED heap_update (final HOT): rel=%s otid=(%u,%u) hot_allowed=%d newbuf==buffer=%d
use_hot_update=%d",
4107-         RelationGetRelationName(relation),
4108-         ItemPointerGetBlockNumber(otid),
4109-         ItemPointerGetOffsetNumber(otid),
4110-         hot_allowed, (newbuf == buffer), use_hot_update);
4111-
--
4693:    elog(LOG, "PATCHED simple_heap_update: rel=%s otid=(%u,%u) hot_allowed=%d summarized_only=%d lockmode=%d",
4694-         RelationGetRelationName(relation),
4695-         ItemPointerGetBlockNumber(otid),
4696-         ItemPointerGetOffsetNumber(otid),
4697-         hot_allowed, summarized_only, lockmode);
4698-

                                             
src/backend/access/heap/heapam_handler.c
333:    elog(LOG, "PATCHED heapam_tuple_update: rel=%s otid=(%u,%u) hot_allowed=%d summarized_only=%d lockmode=%d",
334-         RelationGetRelationName(relation),
335-         ItemPointerGetBlockNumber(otid),
336-         ItemPointerGetOffsetNumber(otid),
337-         hot_allowed, summarized_only, *lockmode);