public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v3 07/17] Execute freezing in heap_page_prune() 10+ messages / 4 participants [nested] [flat]
* [PATCH v3 07/17] Execute freezing in heap_page_prune() @ 2024-03-08 21:45 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 10+ messages in thread From: Melanie Plageman @ 2024-03-08 21:45 UTC (permalink / raw) As a step toward combining the prune and freeze WAL records, execute freezing in heap_page_prune(). The logic to determine whether or not to execute freeze plans was moved from lazy_scan_prune() over to heap_page_prune() with little modification. --- src/backend/access/heap/heapam_handler.c | 2 +- src/backend/access/heap/pruneheap.c | 151 +++++++++++++++++------ src/backend/access/heap/vacuumlazy.c | 129 ++++++------------- src/backend/storage/ipc/procarray.c | 6 +- src/include/access/heapam.h | 41 +++--- src/tools/pgindent/typedefs.list | 2 +- 6 files changed, 180 insertions(+), 151 deletions(-) diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 680a50bf8b1..5e522f5b0ba 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -1046,7 +1046,7 @@ heapam_scan_analyze_next_tuple(TableScanDesc scan, TransactionId OldestXmin, * We ignore unused and redirect line pointers. DEAD line pointers * should be counted as dead, because we need vacuum to run to get rid * of them. Note that this rule agrees with the way that - * heap_page_prune() counts things. + * heap_page_prune_and_freeze() counts things. */ if (!ItemIdIsNormal(itemid)) { diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c index 6bd8400b33b..abf6bdb2d99 100644 --- a/src/backend/access/heap/pruneheap.c +++ b/src/backend/access/heap/pruneheap.c @@ -17,16 +17,18 @@ #include "access/heapam.h" #include "access/heapam_xlog.h" #include "access/htup_details.h" +#include "access/multixact.h" #include "access/transam.h" #include "access/xlog.h" #include "access/xloginsert.h" +#include "executor/instrument.h" #include "miscadmin.h" #include "pgstat.h" #include "storage/bufmgr.h" #include "utils/rel.h" #include "utils/snapmgr.h" -/* Working data for heap_page_prune and subroutines */ +/* Working data for heap_page_prune_and_freeze() and subroutines */ typedef struct { Relation rel; @@ -61,17 +63,18 @@ static HTSV_Result heap_prune_satisfies_vacuum(PruneState *prstate, Buffer buffer); static int heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum, - PruneState *prstate, PruneResult *presult); + PruneState *prstate, PruneFreezeResult *presult); static void prune_prepare_freeze_tuple(Page page, OffsetNumber offnum, - HeapPageFreeze *pagefrz, PruneResult *presult); + HeapPageFreeze *pagefrz, HeapTupleFreeze *frozen, + PruneFreezeResult *presult); static void heap_prune_record_prunable(PruneState *prstate, TransactionId xid); static void heap_prune_record_redirect(PruneState *prstate, OffsetNumber offnum, OffsetNumber rdoffnum); static void heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum, - PruneResult *presult); + PruneFreezeResult *presult); static void heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum, - PruneResult *presult); + PruneFreezeResult *presult); static void heap_prune_record_unused(PruneState *prstate, OffsetNumber offnum); static void page_verify_redirects(Page page); @@ -151,15 +154,15 @@ heap_page_prune_opt(Relation relation, Buffer buffer) */ if (PageIsFull(page) || PageGetHeapFreeSpace(page) < minfree) { - PruneResult presult; + PruneFreezeResult presult; /* * For now, pass mark_unused_now as false regardless of whether or * not the relation has indexes, since we cannot safely determine * that during on-access pruning with the current implementation. */ - heap_page_prune(relation, buffer, vistest, false, NULL, - &presult, NULL); + heap_page_prune_and_freeze(relation, buffer, vistest, false, NULL, + &presult, NULL); /* * Report the number of tuples reclaimed to pgstats. This is @@ -207,7 +210,12 @@ prune_freeze_xmin_is_removable(GlobalVisState *visstate, TransactionId xmin) } /* - * Prune and repair fragmentation in the specified page. + * Prune and repair fragmentation and potentially freeze tuples on the + * specified page. + * + * If the page can be marked all-frozen in the visibility map, we may + * opportunistically freeze tuples on the page if either its tuples are old + * enough or freezing will be cheap enough. * * Caller must have pin and buffer cleanup lock on the page. Note that we * don't update the FSM information for page on caller's behalf. Caller might @@ -221,23 +229,24 @@ prune_freeze_xmin_is_removable(GlobalVisState *visstate, TransactionId xmin) * mark_unused_now indicates whether or not dead items can be set LP_UNUSED during * pruning. * - * pagefrz contains both input and output parameters used if the caller is - * interested in potentially freezing tuples on the page. + * pagefrz is an input parameter containing visibility cutoff information and + * the current relfrozenxid and relminmxids used if the caller is interested in + * freezing tuples on the page. * * off_loc is the offset location required by the caller to use in error * callback. * * presult contains output parameters needed by callers such as the number of * tuples removed and the number of line pointers newly marked LP_DEAD. - * heap_page_prune() is responsible for initializing it. + * heap_page_prune_and_freeze() is responsible for initializing it. */ void -heap_page_prune(Relation relation, Buffer buffer, - GlobalVisState *vistest, - bool mark_unused_now, - HeapPageFreeze *pagefrz, - PruneResult *presult, - OffsetNumber *off_loc) +heap_page_prune_and_freeze(Relation relation, Buffer buffer, + GlobalVisState *vistest, + bool mark_unused_now, + HeapPageFreeze *pagefrz, + PruneFreezeResult *presult, + OffsetNumber *off_loc) { Page page = BufferGetPage(buffer); BlockNumber blockno = BufferGetBlockNumber(buffer); @@ -245,6 +254,14 @@ heap_page_prune(Relation relation, Buffer buffer, maxoff; PruneState prstate; HeapTupleData tup; + bool do_freeze; + int64 fpi_before = pgWalUsage.wal_fpi; + TransactionId frz_conflict_horizon = InvalidTransactionId; + + /* + * One entry for every tuple that we may freeze. + */ + HeapTupleFreeze frozen[MaxHeapTuplesPerPage]; /* * Our strategy is to scan the page and make lists of items to change, @@ -281,6 +298,10 @@ heap_page_prune(Relation relation, Buffer buffer, /* for recovery conflicts */ presult->frz_conflict_horizon = InvalidTransactionId; + /* For advancing relfrozenxid and relminmxid */ + presult->new_relfrozenxid = InvalidTransactionId; + presult->new_relminmxid = InvalidMultiXactId; + maxoff = PageGetMaxOffsetNumber(page); tup.t_tableOid = RelationGetRelid(prstate.rel); @@ -440,7 +461,7 @@ heap_page_prune(Relation relation, Buffer buffer, if (pagefrz) prune_prepare_freeze_tuple(page, offnum, - pagefrz, presult); + pagefrz, frozen, presult); /* Ignore items already processed as part of an earlier chain */ if (prstate.marked[offnum]) @@ -555,6 +576,61 @@ heap_page_prune(Relation relation, Buffer buffer, /* Record number of newly-set-LP_DEAD items for caller */ presult->nnewlpdead = prstate.ndead; + + /* + * Freeze the page when heap_prepare_freeze_tuple indicates that at least + * one XID/MXID from before FreezeLimit/MultiXactCutoff is present. Also + * freeze when pruning generated an FPI, if doing so means that we set the + * page all-frozen afterwards (might not happen until final heap pass). + */ + if (pagefrz) + do_freeze = pagefrz->freeze_required || + (presult->all_visible_except_removable && presult->all_frozen && + presult->nfrozen > 0 && + fpi_before != pgWalUsage.wal_fpi); + else + do_freeze = false; + + if (do_freeze) + { + frz_conflict_horizon = heap_frz_conflict_horizon(presult, pagefrz); + + /* Execute all freeze plans for page as a single atomic action */ + heap_freeze_execute_prepared(relation, buffer, + frz_conflict_horizon, + frozen, presult->nfrozen); + } + else if (!pagefrz || !presult->all_frozen || presult->nfrozen > 0) + { + /* + * If we will neither freeze tuples on the page nor set the page all + * frozen in the visibility map, the page is not all frozen and there + * will be no newly frozen tuples. + */ + presult->all_frozen = false; + presult->nfrozen = 0; /* avoid miscounts in instrumentation */ + } + + /* Caller won't update new_relfrozenxid and new_relminmxid */ + if (!pagefrz) + return; + + /* + * If we will freeze tuples on the page or, even if we don't freeze tuples + * on the page, if we will set the page all-frozen in the visibility map, + * we can advance relfrozenxid and relminmxid to the values in + * pagefrz->FreezePageRelfrozenXid and pagefrz->FreezePageRelminMxid. + */ + if (presult->all_frozen || presult->nfrozen > 0) + { + presult->new_relfrozenxid = pagefrz->FreezePageRelfrozenXid; + presult->new_relminmxid = pagefrz->FreezePageRelminMxid; + } + else + { + presult->new_relfrozenxid = pagefrz->NoFreezePageRelfrozenXid; + presult->new_relminmxid = pagefrz->NoFreezePageRelminMxid; + } } @@ -612,7 +688,7 @@ heap_prune_satisfies_vacuum(PruneState *prstate, HeapTuple tup, Buffer buffer) */ static int heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum, - PruneState *prstate, PruneResult *presult) + PruneState *prstate, PruneFreezeResult *presult) { int ndeleted = 0; Page dp = (Page) BufferGetPage(buffer); @@ -877,10 +953,10 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum, { /* * We found a redirect item that doesn't point to a valid follow-on - * item. This can happen if the loop in heap_page_prune caused us to - * visit the dead successor of a redirect item before visiting the - * redirect item. We can clean up by setting the redirect item to - * DEAD state or LP_UNUSED if the caller indicated. + * item. This can happen if the loop in heap_page_prune_and_freeze() + * caused us to visit the dead successor of a redirect item before + * visiting the redirect item. We can clean up by setting the + * redirect item to DEAD state or LP_UNUSED if the caller indicated. */ heap_prune_record_dead_or_unused(prstate, rootoffnum, presult); } @@ -897,7 +973,8 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum, static void prune_prepare_freeze_tuple(Page page, OffsetNumber offnum, HeapPageFreeze *pagefrz, - PruneResult *presult) + HeapTupleFreeze *frozen, + PruneFreezeResult *presult) { bool totally_frozen; HeapTupleHeader htup; @@ -919,11 +996,11 @@ prune_prepare_freeze_tuple(Page page, OffsetNumber offnum, /* Tuple with storage -- consider need to freeze */ if ((heap_prepare_freeze_tuple(htup, pagefrz, - &presult->frozen[presult->nfrozen], + &frozen[presult->nfrozen], &totally_frozen))) { /* Save prepared freeze plan for later */ - presult->frozen[presult->nfrozen++].offset = offnum; + frozen[presult->nfrozen++].offset = offnum; } /* @@ -967,7 +1044,7 @@ heap_prune_record_redirect(PruneState *prstate, /* Record line pointer to be marked dead */ static void heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum, - PruneResult *presult) + PruneFreezeResult *presult) { Assert(prstate->ndead < MaxHeapTuplesPerPage); prstate->nowdead[prstate->ndead] = offnum; @@ -990,7 +1067,7 @@ heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum, */ static void heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum, - PruneResult *presult) + PruneFreezeResult *presult) { /* * If the caller set mark_unused_now to true, we can remove dead tuples @@ -1017,9 +1094,9 @@ heap_prune_record_unused(PruneState *prstate, OffsetNumber offnum) /* - * Perform the actual page changes needed by heap_page_prune. - * It is expected that the caller has a full cleanup lock on the - * buffer. + * Perform the actual page pruning modifications needed by + * heap_page_prune_and_freeze(). It is expected that the caller has a full + * cleanup lock on the buffer. */ void heap_page_prune_execute(Buffer buffer, @@ -1133,11 +1210,11 @@ heap_page_prune_execute(Buffer buffer, #ifdef USE_ASSERT_CHECKING /* - * When heap_page_prune() was called, mark_unused_now may have been - * passed as true, which allows would-be LP_DEAD items to be made - * LP_UNUSED instead. This is only possible if the relation has no - * indexes. If there are any dead items, then mark_unused_now was not - * true and every item being marked LP_UNUSED must refer to a + * When heap_page_prune_and_freeze() was called, mark_unused_now may + * have been passed as true, which allows would-be LP_DEAD items to be + * made LP_UNUSED instead. This is only possible if the relation has + * no indexes. If there are any dead items, then mark_unused_now was + * not true and every item being marked LP_UNUSED must refer to a * heap-only tuple. */ if (ndead > 0) diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index abbb7ab3ada..6dd8d457c9c 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -269,9 +269,6 @@ static void update_vacuum_error_info(LVRelState *vacrel, static void restore_vacuum_error_info(LVRelState *vacrel, const LVSavedErrInfo *saved_vacrel); -static TransactionId heap_frz_conflict_horizon(PruneResult *presult, - HeapPageFreeze *pagefrz); - /* * heap_vacuum_rel() -- perform VACUUM for one heap relation * @@ -432,12 +429,13 @@ heap_vacuum_rel(Relation rel, VacuumParams *params, * as an upper bound on the XIDs stored in the pages we'll actually scan * (NewRelfrozenXid tracking must never be allowed to miss unfrozen XIDs). * - * Next acquire vistest, a related cutoff that's used in heap_page_prune. - * We expect vistest will always make heap_page_prune remove any deleted - * tuple whose xmax is < OldestXmin. lazy_scan_prune must never become - * confused about whether a tuple should be frozen or removed. (In the - * future we might want to teach lazy_scan_prune to recompute vistest from - * time to time, to increase the number of dead tuples it can prune away.) + * Next acquire vistest, a related cutoff that's used in + * heap_page_prune_and_freeze(). We expect vistest will always make + * heap_page_prune_and_freeze() remove any deleted tuple whose xmax is < + * OldestXmin. lazy_scan_prune must never become confused about whether a + * tuple should be frozen or removed. (In the future we might want to + * teach lazy_scan_prune to recompute vistest from time to time, to + * increase the number of dead tuples it can prune away.) */ vacrel->aggressive = vacuum_get_cutoffs(rel, params, &vacrel->cutoffs); vacrel->rel_pages = orig_rel_pages = RelationGetNumberOfBlocks(rel); @@ -1379,8 +1377,8 @@ lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf, BlockNumber blkno, * Determine the snapshotConflictHorizon for freezing. Must only be called * after pruning and determining if the page is freezable. */ -static TransactionId -heap_frz_conflict_horizon(PruneResult *presult, HeapPageFreeze *pagefrz) +TransactionId +heap_frz_conflict_horizon(PruneFreezeResult *presult, HeapPageFreeze *pagefrz) { TransactionId result; @@ -1407,21 +1405,21 @@ heap_frz_conflict_horizon(PruneResult *presult, HeapPageFreeze *pagefrz) * * Caller must hold pin and buffer cleanup lock on the buffer. * - * Prior to PostgreSQL 14 there were very rare cases where heap_page_prune() - * was allowed to disagree with our HeapTupleSatisfiesVacuum() call about - * whether or not a tuple should be considered DEAD. This happened when an - * inserting transaction concurrently aborted (after our heap_page_prune() - * call, before our HeapTupleSatisfiesVacuum() call). There was rather a lot - * of complexity just so we could deal with tuples that were DEAD to VACUUM, - * but nevertheless were left with storage after pruning. + * Prior to PostgreSQL 14 there were very rare cases where + * heap_page_prune_and_freeze() was allowed to disagree with our + * HeapTupleSatisfiesVacuum() call about whether or not a tuple should be + * considered DEAD. This happened when an inserting transaction concurrently + * aborted (after our heap_page_prune_and_freeze() call, before our + * HeapTupleSatisfiesVacuum() call). There was rather a lot of complexity just + * so we could deal with tuples that were DEAD to VACUUM, but nevertheless were + * left with storage after pruning. * * As of Postgres 17, we circumvent this problem altogether by reusing the - * result of heap_page_prune()'s visibility check. Without the second call to - * HeapTupleSatisfiesVacuum(), there is no new HTSV_Result and there can be no - * disagreement. We'll just handle such tuples as if they had become fully dead - * right after this operation completes instead of in the middle of it. Note that - * any tuple that becomes dead after the call to heap_page_prune() can't need to - * be frozen, because it was visible to another session when vacuum started. + * result of heap_page_prune_and_freeze()'s visibility check. Without the + * second call to HeapTupleSatisfiesVacuum(), there is no new HTSV_Result and + * there can be no disagreement. We'll just handle such tuples as if they had + * become fully dead right after this operation completes instead of in the + * middle of it. * * vmbuffer is the buffer containing the VM block with visibility information * for the heap block, blkno. all_visible_according_to_vm is the saved @@ -1444,26 +1442,24 @@ lazy_scan_prune(LVRelState *vacrel, OffsetNumber offnum, maxoff; ItemId itemid; - PruneResult presult; + PruneFreezeResult presult; int lpdead_items, live_tuples, recently_dead_tuples; HeapPageFreeze pagefrz; bool hastup = false; - bool do_freeze; - int64 fpi_before = pgWalUsage.wal_fpi; OffsetNumber deadoffsets[MaxHeapTuplesPerPage]; Assert(BufferGetBlockNumber(buf) == blkno); /* * maxoff might be reduced following line pointer array truncation in - * heap_page_prune. That's safe for us to ignore, since the reclaimed - * space will continue to look like LP_UNUSED items below. + * heap_page_prune_and_freeze(). That's safe for us to ignore, since the + * reclaimed space will continue to look like LP_UNUSED items below. */ maxoff = PageGetMaxOffsetNumber(page); - /* Initialize (or reset) page-level state */ + /* Initialize pagefrz */ pagefrz.freeze_required = false; pagefrz.FreezePageRelfrozenXid = vacrel->NewRelfrozenXid; pagefrz.FreezePageRelminMxid = vacrel->NewRelminMxid; @@ -1475,7 +1471,7 @@ lazy_scan_prune(LVRelState *vacrel, recently_dead_tuples = 0; /* - * Prune all HOT-update chains in this page. + * Prune all HOT-update chains and potentially freeze tuples on this page. * * We count the number of tuples removed from the page by the pruning step * in presult.ndeleted. It should not be confused with lpdead_items; @@ -1486,8 +1482,8 @@ lazy_scan_prune(LVRelState *vacrel, * items LP_UNUSED, so mark_unused_now should be true if no indexes and * false otherwise. */ - heap_page_prune(rel, buf, vacrel->vistest, vacrel->nindexes == 0, - &pagefrz, &presult, &vacrel->offnum); + heap_page_prune_and_freeze(rel, buf, vacrel->vistest, vacrel->nindexes == 0, + &pagefrz, &presult, &vacrel->offnum); /* * Now scan the page to collect LP_DEAD items and check for tuples @@ -1604,72 +1600,23 @@ lazy_scan_prune(LVRelState *vacrel, vacrel->offnum = InvalidOffsetNumber; - /* - * Freeze the page when heap_prepare_freeze_tuple indicates that at least - * one XID/MXID from before FreezeLimit/MultiXactCutoff is present. Also - * freeze when pruning generated an FPI, if doing so means that we set the - * page all-frozen afterwards (might not happen until final heap pass). - */ - do_freeze = pagefrz.freeze_required || - (presult.all_visible_except_removable && presult.all_frozen && - presult.nfrozen > 0 && - fpi_before != pgWalUsage.wal_fpi); + Assert(MultiXactIdIsValid(presult.new_relminmxid)); + vacrel->NewRelfrozenXid = presult.new_relfrozenxid; + Assert(TransactionIdIsValid(presult.new_relfrozenxid)); + vacrel->NewRelminMxid = presult.new_relminmxid; - if (do_freeze) + if (presult.nfrozen > 0) { - TransactionId snapshotConflictHorizon; - /* - * We're freezing the page. Our final NewRelfrozenXid doesn't need to - * be affected by the XIDs that are just about to be frozen anyway. + * We never increment the frozen_pages instrumentation counter when + * nfrozen == 0, since it only counts pages with newly frozen tuples + * (don't confuse that with pages newly set all-frozen in VM). */ - vacrel->NewRelfrozenXid = pagefrz.FreezePageRelfrozenXid; - vacrel->NewRelminMxid = pagefrz.FreezePageRelminMxid; - vacrel->frozen_pages++; - snapshotConflictHorizon = heap_frz_conflict_horizon(&presult, &pagefrz); - /* Using same cutoff when setting VM is now unnecessary */ - if (presult.all_visible_except_removable && presult.all_frozen) + if (presult.all_frozen) presult.frz_conflict_horizon = InvalidTransactionId; - - /* Execute all freeze plans for page as a single atomic action */ - heap_freeze_execute_prepared(vacrel->rel, buf, - snapshotConflictHorizon, - presult.frozen, presult.nfrozen); - } - else if (presult.all_frozen && presult.nfrozen == 0) - { - /* Page should be all visible except to-be-removed tuples */ - Assert(presult.all_visible_except_removable); - - /* - * We have no freeze plans to execute, so there's no added cost from - * following the freeze path. That's why it was chosen. This is - * important in the case where the page only contains totally frozen - * tuples at this point (perhaps only following pruning). Such pages - * can be marked all-frozen in the VM by our caller, even though none - * of its tuples were newly frozen here (note that the "no freeze" - * path never sets pages all-frozen). - * - * We never increment the frozen_pages instrumentation counter here, - * since it only counts pages with newly frozen tuples (don't confuse - * that with pages newly set all-frozen in VM). - */ - vacrel->NewRelfrozenXid = pagefrz.FreezePageRelfrozenXid; - vacrel->NewRelminMxid = pagefrz.FreezePageRelminMxid; - } - else - { - /* - * Page requires "no freeze" processing. It might be set all-visible - * in the visibility map, but it can never be set all-frozen. - */ - vacrel->NewRelfrozenXid = pagefrz.NoFreezePageRelfrozenXid; - vacrel->NewRelminMxid = pagefrz.NoFreezePageRelminMxid; - presult.all_frozen = false; - presult.nfrozen = 0; /* avoid miscounts in instrumentation */ } /* diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c index b3cd248fb64..88a6d504dff 100644 --- a/src/backend/storage/ipc/procarray.c +++ b/src/backend/storage/ipc/procarray.c @@ -1715,9 +1715,9 @@ TransactionIdIsActive(TransactionId xid) * Note: the approximate horizons (see definition of GlobalVisState) are * updated by the computations done here. That's currently required for * correctness and a small optimization. Without doing so it's possible that - * heap vacuum's call to heap_page_prune() uses a more conservative horizon - * than later when deciding which tuples can be removed - which the code - * doesn't expect (breaking HOT). + * heap vacuum's call to heap_page_prune_and_freeze() uses a more conservative + * horizon than later when deciding which tuples can be removed - which the + * code doesn't expect (breaking HOT). */ static void ComputeXidHorizons(ComputeXidHorizonsResult *h) diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h index 2339abfd28a..45c4ae22e6a 100644 --- a/src/include/access/heapam.h +++ b/src/include/access/heapam.h @@ -195,7 +195,7 @@ typedef struct HeapPageFreeze /* * Per-page state returned from pruning */ -typedef struct PruneResult +typedef struct PruneFreezeResult { int ndeleted; /* Number of tuples deleted from the page */ int nnewlpdead; /* Number of newly LP_DEAD items */ @@ -210,9 +210,10 @@ typedef struct PruneResult /* * Tuple visibility is only computed once for each tuple, for correctness - * and efficiency reasons; see comment in heap_page_prune() for details. - * This is of type int8[], instead of HTSV_Result[], so we can use -1 to - * indicate no visibility has been computed, e.g. for LP_DEAD items. + * and efficiency reasons; see comment in heap_page_prune_and_freeze() for + * details. This is of type int8[], instead of HTSV_Result[], so we can + * use -1 to indicate no visibility has been computed, e.g. for LP_DEAD + * items. * * This needs to be MaxHeapTuplesPerPage + 1 long as FirstOffsetNumber is * 1. Otherwise every access would need to subtract 1. @@ -220,17 +221,18 @@ typedef struct PruneResult int8 htsv[MaxHeapTuplesPerPage + 1]; - /* - * One entry for every tuple that we may freeze. - */ - HeapTupleFreeze frozen[MaxHeapTuplesPerPage]; -} PruneResult; + /* New value of relfrozenxid found by heap_page_prune_and_freeze() */ + TransactionId new_relfrozenxid; + + /* New value of relminmxid found by heap_page_prune_and_freeze() */ + MultiXactId new_relminmxid; +} PruneFreezeResult; /* * Pruning calculates tuple visibility once and saves the results in an array - * of int8. See PruneResult.htsv for details. This helper function is meant to - * guard against examining visibility status array members which have not yet - * been computed. + * of int8. See PruneFreezeResult.htsv for details. This helper function is + * meant to guard against examining visibility status array members which have + * not yet been computed. */ static inline HTSV_Result htsv_get_valid_status(int status) @@ -306,6 +308,9 @@ extern TM_Result heap_lock_tuple(Relation relation, HeapTuple tuple, Buffer *buffer, struct TM_FailureData *tmfd); extern void heap_inplace_update(Relation relation, HeapTuple tuple); + +extern TransactionId heap_frz_conflict_horizon(PruneFreezeResult *presult, + HeapPageFreeze *pagefrz); extern bool heap_prepare_freeze_tuple(HeapTupleHeader tuple, HeapPageFreeze *pagefrz, HeapTupleFreeze *frz, bool *totally_frozen); @@ -332,12 +337,12 @@ extern TransactionId heap_index_delete_tuples(Relation rel, /* in heap/pruneheap.c */ struct GlobalVisState; extern void heap_page_prune_opt(Relation relation, Buffer buffer); -extern void heap_page_prune(Relation relation, Buffer buffer, - struct GlobalVisState *vistest, - bool mark_unused_now, - HeapPageFreeze *pagefrz, - PruneResult *presult, - OffsetNumber *off_loc); +extern void heap_page_prune_and_freeze(Relation relation, Buffer buffer, + struct GlobalVisState *vistest, + bool mark_unused_now, + HeapPageFreeze *pagefrz, + PruneFreezeResult *presult, + OffsetNumber *off_loc); extern void heap_page_prune_execute(Buffer buffer, OffsetNumber *redirected, int nredirected, OffsetNumber *nowdead, int ndead, diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index aa7a25b8f8c..1c1a4d305d6 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -2175,7 +2175,7 @@ ProjectionPath PromptInterruptContext ProtocolVersion PrsStorage -PruneResult +PruneFreezeResult PruneState PruneStepResult PsqlScanCallbacks -- 2.40.1 --racicctn4wry6xe5 Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v3-0008-Make-opp-freeze-heuristic-compatible-with-prune-f.patch" ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: RFC: Additional Directory for Extensions @ 2025-04-24 22:59 David E. Wheeler <[email protected]> 0 siblings, 1 reply; 10+ messages in thread From: David E. Wheeler @ 2025-04-24 22:59 UTC (permalink / raw) To: Peter Eisentraut <[email protected]>; +Cc: Matheus Alcantara <[email protected]>; Andrew Dunstan <[email protected]>; Gabriele Bartolini <[email protected]>; Craig Ringer <[email protected]>; Robert Haas <[email protected]>; Alvaro Herrera <[email protected]>; pgsql-hackers On Mar 19, 2025, at 13:29, David E. Wheeler <[email protected]> wrote: > I think this should at least be documented, but generally feels unexpected to me. I’ve attached a patch that fleshes out the docs, along with an example of setting `extension_control_path` and `dynamic_library_path` to use the locations. It might not have the information right about the need for “postgresql” or “pgsql” in the path. Back in 2003[1] it was just “postgres”, but I couldn’t find the logic for it just now. Here’s a rebase. Best, David Attachments: [application/octet-stream] v2-0001-Flesh-out-docs-for-the-prefix-make-variable.patch (2.6K, ../../[email protected]/2-v2-0001-Flesh-out-docs-for-the-prefix-make-variable.patch) download | inline diff: From 448a03ac53fd145eb5da63c96ac99a99876de642 Mon Sep 17 00:00:00 2001 From: "David E. Wheeler" <[email protected]> Date: Thu, 24 Apr 2025 18:57:26 -0400 Subject: [PATCH v2] Flesh out docs for the `prefix` make variable The variable is a bit magical in how it requires "postgresql" or "pgsql" to be part of the path, and files end up in its `share` and `lib` subdirectories. So mention all that and show an example of setting `extension_control_path` and `dynamic_library_path` to use those locations. --- doc/src/sgml/extend.sgml | 33 +++++++++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml index 64f8e133cae..4e75a01fae4 100644 --- a/doc/src/sgml/extend.sgml +++ b/doc/src/sgml/extend.sgml @@ -1809,10 +1809,35 @@ include $(PGXS) setting <varname>PG_CONFIG</varname> to point to its <command>pg_config</command> program, either within the makefile or on the <literal>make</literal> command line. - You can also select a separate installation directory for your extension - by setting the <literal>make</literal> variable <varname>prefix</varname> - on the <literal>make</literal> command line. (But this will then require - additional setup to get the server to find the extension there.) + </para> + + <para> + You can also select a separate directory prefix in which to install your + extension's files by setting the <literal>make</literal> variable + <varname>prefix</varname> when executing <literal>make install</literal> + like so: +<programlisting> +make install prefix=/etc/postgresql +</programlisting> + This will install the control SQL files into + <literal>/etc/postgresql/share</literal> and shared modules into + <literal>/etc/postgresql/lib</literal>. If the prefix does not + include the strings <literal>postgresql</literal> or + <literal>pgsql</literal>, such as: +<programlisting> +make install prefix=/etc/extras +</programlisting> + Then the <literal>postgresql</literal> directory will be appended io the + prefix, installing the control SQL files into + <literal>/etc/extras/postgresql/share</literal> and shared modules into + <literal>/etc/extras/postgresql/lib</literal>. Either way, you'll need to + set <xref linkend="guc-extension-control-path"/> and <xref + linkend="guc-dynamic-library-path"/> to allow + <productname>PostgreSQL</productname> to find the files: +</programlisting> +extension_control_path = '/etc/extras/postgresql/share/extension:$system' +dynamic_library_path = '/etc/extras/postgresql/lib:$libdir' + </programlisting> </para> <para> -- 2.48.1 [application/pgp-signature] signature.asc (833B, ../../[email protected]/3-signature.asc) download ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: RFC: Additional Directory for Extensions @ 2025-04-25 11:33 Christoph Berg <[email protected]> parent: David E. Wheeler <[email protected]> 0 siblings, 1 reply; 10+ messages in thread From: Christoph Berg @ 2025-04-25 11:33 UTC (permalink / raw) To: David E. Wheeler <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Matheus Alcantara <[email protected]>; Andrew Dunstan <[email protected]>; Gabriele Bartolini <[email protected]>; Craig Ringer <[email protected]>; Robert Haas <[email protected]>; Alvaro Herrera <[email protected]>; pgsql-hackers Re: David E. Wheeler > +<programlisting> > +make install prefix=/etc/postgresql I'd use /usr/local/postgresql there. "/etc" is just wrong. > +</programlisting> > + This will install the control SQL files into > + <literal>/etc/postgresql/share</literal> and shared modules into > + <literal>/etc/postgresql/lib</literal>. If the prefix does not > + include the strings <literal>postgresql</literal> or Just "postgres", see src/Makefile.global.in:86. > + <literal>pgsql</literal>, such as: > +<programlisting> > +make install prefix=/etc/extras /usr/local/extras > +</programlisting> > + Then the <literal>postgresql</literal> directory will be appended io the > + prefix, installing the control SQL files into "the extension control and SQL files" > + <literal>/etc/extras/postgresql/share</literal> and shared modules into .../postgresql/share/extension because ... > + <literal>/etc/extras/postgresql/lib</literal>. Either way, you'll need to > + set <xref linkend="guc-extension-control-path"/> and <xref > + linkend="guc-dynamic-library-path"/> to allow > + <productname>PostgreSQL</productname> to find the files: > +</programlisting> > +extension_control_path = '/etc/extras/postgresql/share/extension:$system' ... it's used here. Christoph ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: RFC: Additional Directory for Extensions @ 2025-04-25 19:23 David E. Wheeler <[email protected]> parent: Christoph Berg <[email protected]> 0 siblings, 1 reply; 10+ messages in thread From: David E. Wheeler @ 2025-04-25 19:23 UTC (permalink / raw) To: Christoph Berg <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Matheus Alcantara <[email protected]>; Andrew Dunstan <[email protected]>; Gabriele Bartolini <[email protected]>; Craig Ringer <[email protected]>; Robert Haas <[email protected]>; Alvaro Herrera <[email protected]>; pgsql-hackers On Apr 25, 2025, at 07:33, Christoph Berg <[email protected]> wrote: > > Re: David E. Wheeler >> +<programlisting> >> +make install prefix=/etc/postgresql > > I'd use /usr/local/postgresql there. "/etc" is just wrong. Thank you for the review. Here’s v3*. Best, David * Also reviewable as a GitHub PR[1]. [1]: https://github.com/theory/postgres/pull/10 Attachments: [application/octet-stream] v3-0001-Flesh-out-docs-for-the-prefix-make-variable.patch (2.7K, ../../[email protected]/2-v3-0001-Flesh-out-docs-for-the-prefix-make-variable.patch) download | inline diff: From d49d3445ca5bdde436713dc8a2ae7707683851e3 Mon Sep 17 00:00:00 2001 From: "David E. Wheeler" <[email protected]> Date: Fri, 25 Apr 2025 15:22:23 -0400 Subject: [PATCH v3] Flesh out docs for the `prefix` make variable The variable is a bit magical in how it requires "postgresql" or "pgsql" to be part of the path, and files end up in its `share` and `lib` subdirectories. So mention all that and show an example of setting `extension_control_path` and `dynamic_library_path` to use those locations. --- doc/src/sgml/extend.sgml | 33 +++++++++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml index 64f8e133cae..05063d4e7bc 100644 --- a/doc/src/sgml/extend.sgml +++ b/doc/src/sgml/extend.sgml @@ -1809,10 +1809,35 @@ include $(PGXS) setting <varname>PG_CONFIG</varname> to point to its <command>pg_config</command> program, either within the makefile or on the <literal>make</literal> command line. - You can also select a separate installation directory for your extension - by setting the <literal>make</literal> variable <varname>prefix</varname> - on the <literal>make</literal> command line. (But this will then require - additional setup to get the server to find the extension there.) + </para> + + <para> + You can also select a separate directory prefix in which to install your + extension's files by setting the <literal>make</literal> variable + <varname>prefix</varname> when executing <literal>make install</literal> + like so: +<programlisting> +make install prefix=/usr/local/postgresql +</programlisting> + This will install the control SQL files into + <literal>/usr/local/postgresql/share</literal> and shared modules into + <literal>/usr/local/postgresql/lib</literal>. If the prefix does not + include the strings <literal>postgres</literal> or + <literal>pgsql</literal>, such as: +<programlisting> +make install prefix=/usr/local/extras +</programlisting> + Then the <literal>postgresql</literal> directory will be appended io the + prefix, installing the control and SQL files into + <literal>/usr/local/extras/postgresql/share/extension</literal> and shared + modules into <literal>/usr/local/extras/postgresql/lib</literal>. Either + way, you'll need to set <xref linkend="guc-extension-control-path"/> and + <xref linkend="guc-dynamic-library-path"/> to allow + <productname>PostgreSQL</productname> to find the files: +</programlisting> +extension_control_path = '/usr/local/extras/postgresql/share/extension:$system' +dynamic_library_path = '/usr/local/extras/postgresql/lib:$libdir' + </programlisting> </para> <para> -- 2.48.1 [application/pgp-signature] signature.asc (833B, ../../[email protected]/4-signature.asc) download ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: RFC: Additional Directory for Extensions @ 2025-04-28 21:14 David E. Wheeler <[email protected]> parent: David E. Wheeler <[email protected]> 0 siblings, 1 reply; 10+ messages in thread From: David E. Wheeler @ 2025-04-28 21:14 UTC (permalink / raw) To: Christoph Berg <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Matheus Alcantara <[email protected]>; Andrew Dunstan <[email protected]>; Gabriele Bartolini <[email protected]>; Craig Ringer <[email protected]>; Robert Haas <[email protected]>; Alvaro Herrera <[email protected]>; pgsql-hackers On Apr 25, 2025, at 15:23, David E. Wheeler <[email protected]> wrote: > Thank you for the review. Here’s v3*. V4 removes “/extension” from the end of the `extension_control_path` value. Best, David Attachments: [application/octet-stream] v4-0001-Flesh-out-docs-for-the-prefix-make-variable.patch (2.7K, ../../[email protected]/2-v4-0001-Flesh-out-docs-for-the-prefix-make-variable.patch) download | inline diff: From 8ff0470bd6b1110c43e9852c5196139df7b2734e Mon Sep 17 00:00:00 2001 From: "David E. Wheeler" <[email protected]> Date: Mon, 28 Apr 2025 17:13:20 -0400 Subject: [PATCH v4] Flesh out docs for the `prefix` make variable The variable is a bit magical in how it requires "postgresql" or "pgsql" to be part of the path, and files end up in its `share` and `lib` subdirectories. So mention all that and show an example of setting `extension_control_path` and `dynamic_library_path` to use those locations. --- doc/src/sgml/extend.sgml | 33 +++++++++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml index 64f8e133cae..68358a5b15f 100644 --- a/doc/src/sgml/extend.sgml +++ b/doc/src/sgml/extend.sgml @@ -1809,10 +1809,35 @@ include $(PGXS) setting <varname>PG_CONFIG</varname> to point to its <command>pg_config</command> program, either within the makefile or on the <literal>make</literal> command line. - You can also select a separate installation directory for your extension - by setting the <literal>make</literal> variable <varname>prefix</varname> - on the <literal>make</literal> command line. (But this will then require - additional setup to get the server to find the extension there.) + </para> + + <para> + You can also select a separate directory prefix in which to install your + extension's files by setting the <literal>make</literal> variable + <varname>prefix</varname> when executing <literal>make install</literal> + like so: +<programlisting> +make install prefix=/usr/local/postgresql +</programlisting> + This will install the control SQL files into + <literal>/usr/local/postgresql/share</literal> and shared modules into + <literal>/usr/local/postgresql/lib</literal>. If the prefix does not + include the strings <literal>postgres</literal> or + <literal>pgsql</literal>, such as: +<programlisting> +make install prefix=/usr/local/extras +</programlisting> + Then the <literal>postgresql</literal> directory will be appended io the + prefix, installing the control and SQL files into + <literal>/usr/local/extras/postgresql/share/extension</literal> and shared + modules into <literal>/usr/local/extras/postgresql/lib</literal>. Either + way, you'll need to set <xref linkend="guc-extension-control-path"/> and + <xref linkend="guc-dynamic-library-path"/> to allow + <productname>PostgreSQL</productname> to find the files: +</programlisting> +extension_control_path = '/usr/local/extras/postgresql/share:$system' +dynamic_library_path = '/usr/local/extras/postgresql/lib:$libdir' + </programlisting> </para> <para> -- 2.49.0 [application/pgp-signature] signature.asc (833B, ../../[email protected]/3-signature.asc) download ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: RFC: Additional Directory for Extensions @ 2025-05-01 11:50 Peter Eisentraut <[email protected]> parent: David E. Wheeler <[email protected]> 0 siblings, 1 reply; 10+ messages in thread From: Peter Eisentraut @ 2025-05-01 11:50 UTC (permalink / raw) To: David E. Wheeler <[email protected]>; +Cc: Matheus Alcantara <[email protected]>; pgsql-hackers; Christoph Berg <[email protected]> On 28.04.25 23:14, David E. Wheeler wrote: > On Apr 25, 2025, at 15:23, David E. Wheeler <[email protected]> wrote: > >> Thank you for the review. Here’s v3*. > > V4 removes “/extension” from the end of the `extension_control_path` value. The documentation in config.sgml says: Note that the path elements should typically end in <literal>extension</literal> if the normal installation layouts are followed. So I think your change here between v3 and v4 is incorrect. ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: RFC: Additional Directory for Extensions @ 2025-05-01 14:31 David E. Wheeler <[email protected]> parent: Peter Eisentraut <[email protected]> 0 siblings, 1 reply; 10+ messages in thread From: David E. Wheeler @ 2025-05-01 14:31 UTC (permalink / raw) To: Peter Eisentraut <[email protected]>; +Cc: Matheus Alcantara <[email protected]>; pgsql-hackers; Christoph Berg <[email protected]> On May 1, 2025, at 07:50, Peter Eisentraut <[email protected]> wrote: > The documentation in config.sgml says: > > Note that the path elements should typically end in > <literal>extension</literal> if the normal installation layouts are > followed. > > So I think your change here between v3 and v4 is incorrect. Right, sorry, forgot about that, I updated it in anticipation of Matheus’s patch[1] being committed. So v3 is fine for now, but if that patch is committed, we’ll need to reconcile those docs. Best, David [1]: https://postgr.es/m/CAFY6G8dUXHRii5rNy7V8WmBrmBwp9W7y3g+HL6Tn-Lu8KkvK=A@mail.gmail.com Attachments: [application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc) download ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: RFC: Additional Directory for Extensions @ 2025-05-01 20:24 Peter Eisentraut <[email protected]> parent: David E. Wheeler <[email protected]> 0 siblings, 2 replies; 10+ messages in thread From: Peter Eisentraut @ 2025-05-01 20:24 UTC (permalink / raw) To: David E. Wheeler <[email protected]>; +Cc: Matheus Alcantara <[email protected]>; pgsql-hackers; Christoph Berg <[email protected]> On 01.05.25 16:31, David E. Wheeler wrote: > On May 1, 2025, at 07:50, Peter Eisentraut <[email protected]> wrote: > >> The documentation in config.sgml says: >> >> Note that the path elements should typically end in >> <literal>extension</literal> if the normal installation layouts are >> followed. >> >> So I think your change here between v3 and v4 is incorrect. > > Right, sorry, forgot about that, I updated it in anticipation of Matheus’s patch[1] being committed. > > So v3 is fine for now, but if that patch is committed, we’ll need to reconcile those docs. I see. I have committed it now describing the current state. Btw., the shown directory names that illustrate how "postgresql" is appended were not correct. I have corrected that. ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: RFC: Additional Directory for Extensions @ 2025-05-01 21:01 David E. Wheeler <[email protected]> parent: Peter Eisentraut <[email protected]> 1 sibling, 0 replies; 10+ messages in thread From: David E. Wheeler @ 2025-05-01 21:01 UTC (permalink / raw) To: Peter Eisentraut <[email protected]>; +Cc: Matheus Alcantara <[email protected]>; pgsql-hackers; Christoph Berg <[email protected]> On May 1, 2025, at 16:24, Peter Eisentraut <[email protected]> wrote: > I see. I have committed it now describing the current state. > > Btw., the shown directory names that illustrate how "postgresql" is appended were not correct. I have corrected that. Thank you. I gotta say I find them confusing TBH. Best, David Attachments: [application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc) download ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: RFC: Additional Directory for Extensions @ 2025-05-02 16:48 David E. Wheeler <[email protected]> parent: Peter Eisentraut <[email protected]> 1 sibling, 0 replies; 10+ messages in thread From: David E. Wheeler @ 2025-05-02 16:48 UTC (permalink / raw) To: Peter Eisentraut <[email protected]>; +Cc: Matheus Alcantara <[email protected]>; pgsql-hackers; Christoph Berg <[email protected]> On May 1, 2025, at 16:24, Peter Eisentraut <[email protected]> wrote: > I see. I have committed it now describing the current state. Quick follow-up to tweak a couple of commas. --- a/doc/src/sgml/extend.sgml +++ b/doc/src/sgml/extend.sgml @@ -1813,8 +1813,8 @@ include $(PGXS) <para> You can select a separate directory prefix in which to install your - extension's files, by setting the <command>make</command> variable - <varname>prefix</varname> when executing <literal>make install</literal> + extension's files by setting the <command>make</command> variable + <varname>prefix</varname> when executing <literal>make install</literal>, like so: <programlisting> make install prefix=/usr/local/postgresql Best, David Attachments: [application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc) download ^ permalink raw reply [nested|flat] 10+ messages in thread
end of thread, other threads:[~2025-05-02 16:48 UTC | newest] Thread overview: 10+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2024-03-08 21:45 [PATCH v3 07/17] Execute freezing in heap_page_prune() Melanie Plageman <[email protected]> 2025-04-24 22:59 Re: RFC: Additional Directory for Extensions David E. Wheeler <[email protected]> 2025-04-25 11:33 ` Re: RFC: Additional Directory for Extensions Christoph Berg <[email protected]> 2025-04-25 19:23 ` Re: RFC: Additional Directory for Extensions David E. Wheeler <[email protected]> 2025-04-28 21:14 ` Re: RFC: Additional Directory for Extensions David E. Wheeler <[email protected]> 2025-05-01 11:50 ` Re: RFC: Additional Directory for Extensions Peter Eisentraut <[email protected]> 2025-05-01 14:31 ` Re: RFC: Additional Directory for Extensions David E. Wheeler <[email protected]> 2025-05-01 20:24 ` Re: RFC: Additional Directory for Extensions Peter Eisentraut <[email protected]> 2025-05-01 21:01 ` Re: RFC: Additional Directory for Extensions David E. Wheeler <[email protected]> 2025-05-02 16:48 ` Re: RFC: Additional Directory for Extensions David E. Wheeler <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox