Re: eliminate xl_heap_visible to reduce WAL (and eventually set VM on-access)
Melanie Plageman <melanieplageman@gmail.com>
Attachments
- v47-0001-Make-it-cheap-to-check-with-relations-are-modifi.patch (text/x-patch) patch v47-0001
- v47-0002-Remove-PlannedStmt-resultRelations-in-favor-of-r.patch (text/x-patch) patch v47-0002
- v47-0003-Thread-flags-through-begin-scan-APIs.patch (text/x-patch) patch v47-0003
- v47-0004-Pass-down-information-on-table-modification-to-s.patch (text/x-patch) patch v47-0004
- v47-0005-Allow-on-access-pruning-to-set-pages-all-visible.patch (text/x-patch) patch v47-0005
- v47-0006-Set-pd_prune_xid-on-insert.patch (text/x-patch) patch v47-0006
On Thu, Mar 26, 2026 at 7:10 PM David Rowley <dgrowleyml@gmail.com> wrote: > > I was looking at v46-0001. With: Thanks for taking a look! > +++ b/src/include/nodes/plannodes.h > + Bitmapset *modifiedRelids; > + > > This doesn't really mention anything about leaf partitions not being > mentioned for INSERT queries. You did mention it in standard_planner() > here: > > I'm also wondering about having this combined field. If you were to > have a Bitmapset field that mirrors "List *resultRelations;", then > have another: > > /* a list of PlanRowMark's */ > List *rowMarks; > > + /* Relids which have rowMarks */ > + Bitmapset *rowMarkRelids; > > I think they're more likely to be useful for other purposes, and I > think the only pain that it causes you is that you have to call > bms_is_member() twice in ScanRelIsReadOnly(). Yea, outside of the insert into leaf partitions case, I thought of another, perhaps even more compelling reason the combined field might be a bit confusing: Take a table t1 and a table t2. If you do SELECT * FROM t1 JOIN t2 ON t1.id = t2.id FOR UPDATE of t1; t1 will get a ROW_MARK_EXCLUSIVE and t2 would get a ROW_MARK_REFERENCE (that's just how preprocess_rowmarks() works). That means modifiedRelids would contain t2, even though t2 is not being locked for update. For the purposes of setting the VM, it's totally fine that we are more conservative than we need to be and don't consider setting it when scanning t2. But for the purposes of modifiedRelids, it's a bit confusing that t2 is in there. But we can't just exclude ROW_MARK_REFERENCE from modifiedRelids because we rely on ROW_MARK_REFERENCE to avoid setting the VM for a table we are updating or deleting from when it is mentioned more than once in the query (e.g. UPDATE foo SET x = 1 FROM foo f2 WHERE foo.id = f2.id). So, for that reason and because of the missing leaf partitions for inserts, I think making quick reference bitmapsets would be better. I've done this in attached v47. I've also removed the asserts in ExecInsert/Update/Delete because they are a bit tautological now. My one remaining question is whether the two new bitmapsets (rowMarkRelids and resultRelationRelids) should move from the PlannedStmt to the EState. They are determined at plan time and never modified during execution. However, I do notice there are other EState members that seem like just a copy of info from the PlannedStmt that isn't modified during execution (e.g. es_rteperminfos/permInfos). However, putting them in the EState increases the work required to get them to parallel workers and to the child estate for EPQs. I would prefer to keep it in the PlannedStmt but am worried that breaks convention. > Then, as a follow-up, maybe we could consider removing > PlannedStmt.resultRelations. (The deprecated) > ExecRelationIsTargetRelation() could use the new Bitmapset, which > would be more efficient. Yea, I like this and think it makes sense. Done in v47. > I did quickly look over the remaining patches. I wondered if you might > want to add a new ScanOption SO_NONE = 0, or SO_EMPTY_FLAGS. It might > make the places where you're passing zero directly easier to read? That makes sense to me. Done in v47. - Melanie
Commits
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Stabilize btree_gist test against on-access VM setting
- 62407d26b7c9 19 (unreleased) landed
-
Stabilize plancache test against on-access VM setting
- 85ae8ab05334 19 (unreleased) landed
-
Make local buffers pin limit more conservative
- da6874635db2 19 (unreleased) landed
-
Update FSM during prune/freeze replay even if freespace is zero
- b4c1b2be300e 19 (unreleased) landed
-
Pass down information on table modification to scan nodes
- 50eb5faea295 19 (unreleased) landed
-
Thread flags through begin-scan APIs
- dcd8cc1c852c 19 (unreleased) landed
-
Remove PlannedStmt->resultRelations in favor of resultRelationRelids
- 39dcd10a2c49 19 (unreleased) landed
-
Make it cheap to check if a relation is modified by a query
- 0f4c170cf3b8 19 (unreleased) landed
-
Keep newest live XID up-to-date even if page not all-visible
- 9ba3ec076a68 19 (unreleased) landed
-
Add pruning fast path for all-visible and all-frozen pages
- 01b7e4a46d0f 19 (unreleased) landed
-
Detect and fix visibility map corruption in more cases
- 4f7ecca84dda 19 (unreleased) landed
-
Save vmbuffer in heap-specific scan descriptors for on-access pruning
- 99bf1f8aa6cd 19 (unreleased) landed
-
Avoid BufferGetPage() calls in heap_update()
- 8d2c1df4f4c5 19 (unreleased) landed
-
Initialize missing fields in CreateExecutorState()
- a3511443e5e1 19 (unreleased) landed
-
Prefix PruneState->all_{visible,frozen} with set_
- 34cb4254bdb6 19 (unreleased) landed
-
Add PageGetPruneXid() helper
- 68c2dcb9130e 19 (unreleased) landed
-
Move commonly used context into PruneState and simplify helpers
- 59663e4207fd 19 (unreleased) landed
-
Remove table_scan_analyze_next_tuple unneeded parameter OldestXmin
- 284925508ae6 19 (unreleased) landed
-
Simplify visibility check in heap_page_would_be_all_visible()
- 3efe58febc3c 19 (unreleased) landed
-
Rename LVRelState VM-related logging counters
- 5aea60839ba3 19 (unreleased) landed
-
Fix flakiness in the pg_visibility VM-only vacuum test by using a temporary table.
- 4a99ef1a0d11 19 (unreleased) landed
-
Refactor lazy_scan_prune() VM clear logic into helper
- 19af794b660e 19 (unreleased) landed
-
Eliminate use of cached VM value in lazy_scan_prune()
- 648a7e28d7c2 19 (unreleased) landed
-
Combine visibilitymap_set() cases in lazy_scan_prune()
- 21796c267d0a 19 (unreleased) landed
-
Fix const qualification in prune_freeze_setup()
- 4877391ce894 19 (unreleased) landed
-
Simplify vacuum visibility assertion
- bd298f54a0d6 19 (unreleased) landed
-
Split heap_page_prune_and_freeze() into helpers
- e135e044572e 19 (unreleased) landed
-
Assert that cutoffs are provided if freezing will be attempted
- cd38b7e77315 19 (unreleased) landed
-
Split PruneFreezeParams initializers to one field per line
- 1e14edcea5e1 19 (unreleased) landed
-
Refactor heap_page_prune_and_freeze() parameters into a struct
- 1937ed70621e 19 (unreleased) landed
-
Make heap_page_is_all_visible independent of LVRelState
- 3e4705484e0c 19 (unreleased) landed
-
Inline TransactionIdFollows/Precedes[OrEquals]()
- 43b05b38ea4d 19 (unreleased) landed
-
Add helper for freeze determination to heap_page_prune_and_freeze
- c8dd6542bae4 19 (unreleased) landed
-
Bump XLOG_PAGE_MAGIC after xl_heap_prune change
- 4a8fb58671d3 19 (unreleased) landed
-
Correct prune WAL record opcode name in comment
- ae8ea7278c16 19 (unreleased) landed
-
Add error codes when vacuum discovers VM corruption
- 8ec97e78a771 19 (unreleased) landed
-
Remove unused xl_heap_prune member, reason
- 4b5f206de2bb 19 (unreleased) landed
-
Remove unneeded VM pin from VM replay
- 3399c265543e 19 (unreleased) landed
-
Add assert and log message to visibilitymap_set
- e3d5ddb7ca91 19 (unreleased) landed
-
Stamp HEAD as 19devel.
- 2652835d3efa 19 (unreleased) cited
-
Add error codes to some corruption log messages
- fd6ec93bf890 13.0 cited