| From: | "Greg Burd" <greg(at)burd(dot)me> |
|---|---|
| To: | "Jeff Davis" <pgsql(at)j-davis(dot)com> |
| Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: Expanding HOT updates for expression and partial indexes |
| Date: | 2026-03-02 19:08:50 |
| Message-ID: | ff2276c5-28f7-4e09-a6ae-40137ceddb67@app.fastmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | 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 the work.
I thought it might be prudent to add tests that validate all the corner cases of HOT that I could come up with, maybe too many (you tell me). In addition, because code that impacts HOT is also involved in what is WAL logged for the purposes of logical replication, I've added tests that try to explore the corners of that too. The goal of these first few patches is to NOT change the behavior of these things, but to only move the logic into the executor and out of heap then it 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 changing tests 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 comparing master vs patched after instrumenting the code (see below) so I could record HOT and replica identity decisions and 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 on hot_allowed=1 (logic identical), but in some cases use_hot_update differed (buffer placement, newbuf =?= buffer). To me this 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 exactly exciting on the surface, but I feel that it opens the door to other changes that will be more interesting/valuable down 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);
| Attachment | Content-Type | Size |
|---|---|---|
| v33-0001-Add-comprehensive-tests-for-HOT-updates-and-repl.patch | text/x-patch | 102.7 KB |
| v33-0002-Idenfity-modified-indexed-attributes-in-the-exec.patch | text/x-patch | 59.5 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Sami Imseih | 2026-03-02 19:09:07 | Re: Fix bug in multixact Oldest*MXactId initialization and access |
| Previous Message | Sergei Kornilov | 2026-03-02 19:07:16 | Compiler warning when using TRGM_REGEXP_DEBUG |