public inbox for [email protected]help / color / mirror / Atom feed
Commitfest 2023-11 is almost over 14+ messages / 2 participants [nested] [flat]
* Commitfest 2023-11 is almost over @ 2023-12-02 07:23 John Naylor <[email protected]> 0 siblings, 0 replies; 14+ messages in thread From: John Naylor @ 2023-12-02 07:23 UTC (permalink / raw) To: PostgreSQL Hackers <[email protected]> I'm currently spending some time looking for "Needs Review" threads that have some combination of 1) stalled for a long time and 2) lack consensus, with an aim to clearing them out of CF. Since we got a late start, I will begin moving entries over to January on Monday. I thought I remembered discussion on a "bulk move" button, but if it's there, I can't find it... -- John Naylor ^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH v2 06/17] lazy_scan_prune reorder freeze execution logic @ 2024-01-07 19:50 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 14+ messages in thread From: Melanie Plageman @ 2024-01-07 19:50 UTC (permalink / raw) To combine the prune and freeze records, freezing must be done before a pruning WAL record is emitted. We will move the freeze execution into heap_page_prune() in future commits. lazy_scan_prune() currently executes freezing, updates vacrel->NewRelfrozenXid and vacrel->NewRelminMxid, and resets the snapshotConflictHorizon that the visibility map update record may use in the same block of if statements. This commit starts reordering that logic so that the freeze execution can be separated from the other updates which should not be done in pruning. It also adds a helper calculating freeze snapshot conflict horizon. This will be useful when the freeze execution is moved into pruning because not all callers of heap_page_prune() have access to VacuumCutoffs. --- src/backend/access/heap/vacuumlazy.c | 112 ++++++++++++++++----------- 1 file changed, 67 insertions(+), 45 deletions(-) diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index 4187c998d25..abbb7ab3ada 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -269,6 +269,8 @@ 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 @@ -1373,6 +1375,33 @@ lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf, BlockNumber blkno, return false; } +/* + * 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 result; + + /* + * We can use frz_conflict_horizon as our cutoff for conflicts when the + * whole page is eligible to become all-frozen in the VM once we're done + * with it. Otherwise we generate a conservative cutoff by stepping back + * from OldestXmin. + */ + if (presult->all_visible_except_removable && presult->all_frozen) + result = presult->frz_conflict_horizon; + else + { + /* Avoids false conflicts when hot_standby_feedback in use */ + result = pagefrz->cutoffs->OldestXmin; + TransactionIdRetreat(result); + } + + return result; +} + /* * lazy_scan_prune() -- lazy_scan_heap() pruning and freezing. * @@ -1421,6 +1450,7 @@ lazy_scan_prune(LVRelState *vacrel, recently_dead_tuples; HeapPageFreeze pagefrz; bool hastup = false; + bool do_freeze; int64 fpi_before = pgWalUsage.wal_fpi; OffsetNumber deadoffsets[MaxHeapTuplesPerPage]; @@ -1580,10 +1610,15 @@ lazy_scan_prune(LVRelState *vacrel, * 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.freeze_required || presult.nfrozen == 0 || + do_freeze = pagefrz.freeze_required || (presult.all_visible_except_removable && presult.all_frozen && - fpi_before != pgWalUsage.wal_fpi)) + presult.nfrozen > 0 && + fpi_before != pgWalUsage.wal_fpi); + + if (do_freeze) { + 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. @@ -1591,52 +1626,39 @@ lazy_scan_prune(LVRelState *vacrel, vacrel->NewRelfrozenXid = pagefrz.FreezePageRelfrozenXid; vacrel->NewRelminMxid = pagefrz.FreezePageRelminMxid; - if (presult.nfrozen == 0) - { - /* - * 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). - */ - } - else - { - TransactionId snapshotConflictHorizon; + vacrel->frozen_pages++; - vacrel->frozen_pages++; + snapshotConflictHorizon = heap_frz_conflict_horizon(&presult, &pagefrz); - /* - * We can use frz_conflict_horizon as our cutoff for conflicts - * when the whole page is eligible to become all-frozen in the VM - * once we're done with it. Otherwise we generate a conservative - * cutoff by stepping back from OldestXmin. - */ - if (presult.all_visible_except_removable && presult.all_frozen) - { - /* Using same cutoff when setting VM is now unnecessary */ - snapshotConflictHorizon = presult.frz_conflict_horizon; - presult.frz_conflict_horizon = InvalidTransactionId; - } - else - { - /* Avoids false conflicts when hot_standby_feedback in use */ - snapshotConflictHorizon = vacrel->cutoffs.OldestXmin; - TransactionIdRetreat(snapshotConflictHorizon); - } + /* Using same cutoff when setting VM is now unnecessary */ + if (presult.all_visible_except_removable && 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); - } + /* 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 { -- 2.40.1 --rdqtp5puvxqotfdw Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v2-0007-Execute-freezing-in-heap_page_prune.patch" ^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH v3 06/17] lazy_scan_prune reorder freeze execution logic @ 2024-01-07 19:50 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 14+ messages in thread From: Melanie Plageman @ 2024-01-07 19:50 UTC (permalink / raw) To combine the prune and freeze records, freezing must be done before a pruning WAL record is emitted. We will move the freeze execution into heap_page_prune() in future commits. lazy_scan_prune() currently executes freezing, updates vacrel->NewRelfrozenXid and vacrel->NewRelminMxid, and resets the snapshotConflictHorizon that the visibility map update record may use in the same block of if statements. This commit starts reordering that logic so that the freeze execution can be separated from the other updates which should not be done in pruning. It also adds a helper calculating freeze snapshot conflict horizon. This will be useful when the freeze execution is moved into pruning because not all callers of heap_page_prune() have access to VacuumCutoffs. --- src/backend/access/heap/vacuumlazy.c | 112 ++++++++++++++++----------- 1 file changed, 67 insertions(+), 45 deletions(-) diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index 4187c998d25..abbb7ab3ada 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -269,6 +269,8 @@ 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 @@ -1373,6 +1375,33 @@ lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf, BlockNumber blkno, return false; } +/* + * 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 result; + + /* + * We can use frz_conflict_horizon as our cutoff for conflicts when the + * whole page is eligible to become all-frozen in the VM once we're done + * with it. Otherwise we generate a conservative cutoff by stepping back + * from OldestXmin. + */ + if (presult->all_visible_except_removable && presult->all_frozen) + result = presult->frz_conflict_horizon; + else + { + /* Avoids false conflicts when hot_standby_feedback in use */ + result = pagefrz->cutoffs->OldestXmin; + TransactionIdRetreat(result); + } + + return result; +} + /* * lazy_scan_prune() -- lazy_scan_heap() pruning and freezing. * @@ -1421,6 +1450,7 @@ lazy_scan_prune(LVRelState *vacrel, recently_dead_tuples; HeapPageFreeze pagefrz; bool hastup = false; + bool do_freeze; int64 fpi_before = pgWalUsage.wal_fpi; OffsetNumber deadoffsets[MaxHeapTuplesPerPage]; @@ -1580,10 +1610,15 @@ lazy_scan_prune(LVRelState *vacrel, * 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.freeze_required || presult.nfrozen == 0 || + do_freeze = pagefrz.freeze_required || (presult.all_visible_except_removable && presult.all_frozen && - fpi_before != pgWalUsage.wal_fpi)) + presult.nfrozen > 0 && + fpi_before != pgWalUsage.wal_fpi); + + if (do_freeze) { + 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. @@ -1591,52 +1626,39 @@ lazy_scan_prune(LVRelState *vacrel, vacrel->NewRelfrozenXid = pagefrz.FreezePageRelfrozenXid; vacrel->NewRelminMxid = pagefrz.FreezePageRelminMxid; - if (presult.nfrozen == 0) - { - /* - * 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). - */ - } - else - { - TransactionId snapshotConflictHorizon; + vacrel->frozen_pages++; - vacrel->frozen_pages++; + snapshotConflictHorizon = heap_frz_conflict_horizon(&presult, &pagefrz); - /* - * We can use frz_conflict_horizon as our cutoff for conflicts - * when the whole page is eligible to become all-frozen in the VM - * once we're done with it. Otherwise we generate a conservative - * cutoff by stepping back from OldestXmin. - */ - if (presult.all_visible_except_removable && presult.all_frozen) - { - /* Using same cutoff when setting VM is now unnecessary */ - snapshotConflictHorizon = presult.frz_conflict_horizon; - presult.frz_conflict_horizon = InvalidTransactionId; - } - else - { - /* Avoids false conflicts when hot_standby_feedback in use */ - snapshotConflictHorizon = vacrel->cutoffs.OldestXmin; - TransactionIdRetreat(snapshotConflictHorizon); - } + /* Using same cutoff when setting VM is now unnecessary */ + if (presult.all_visible_except_removable && 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); - } + /* 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 { -- 2.40.1 --racicctn4wry6xe5 Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v3-0007-Execute-freezing-in-heap_page_prune.patch" ^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH v3 06/17] lazy_scan_prune reorder freeze execution logic @ 2024-01-07 19:50 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 14+ messages in thread From: Melanie Plageman @ 2024-01-07 19:50 UTC (permalink / raw) To combine the prune and freeze records, freezing must be done before a pruning WAL record is emitted. We will move the freeze execution into heap_page_prune() in future commits. lazy_scan_prune() currently executes freezing, updates vacrel->NewRelfrozenXid and vacrel->NewRelminMxid, and resets the snapshotConflictHorizon that the visibility map update record may use in the same block of if statements. This commit starts reordering that logic so that the freeze execution can be separated from the other updates which should not be done in pruning. It also adds a helper calculating freeze snapshot conflict horizon. This will be useful when the freeze execution is moved into pruning because not all callers of heap_page_prune() have access to VacuumCutoffs. --- src/backend/access/heap/vacuumlazy.c | 112 ++++++++++++++++----------- 1 file changed, 67 insertions(+), 45 deletions(-) diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index 4187c998d25..abbb7ab3ada 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -269,6 +269,8 @@ 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 @@ -1373,6 +1375,33 @@ lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf, BlockNumber blkno, return false; } +/* + * 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 result; + + /* + * We can use frz_conflict_horizon as our cutoff for conflicts when the + * whole page is eligible to become all-frozen in the VM once we're done + * with it. Otherwise we generate a conservative cutoff by stepping back + * from OldestXmin. + */ + if (presult->all_visible_except_removable && presult->all_frozen) + result = presult->frz_conflict_horizon; + else + { + /* Avoids false conflicts when hot_standby_feedback in use */ + result = pagefrz->cutoffs->OldestXmin; + TransactionIdRetreat(result); + } + + return result; +} + /* * lazy_scan_prune() -- lazy_scan_heap() pruning and freezing. * @@ -1421,6 +1450,7 @@ lazy_scan_prune(LVRelState *vacrel, recently_dead_tuples; HeapPageFreeze pagefrz; bool hastup = false; + bool do_freeze; int64 fpi_before = pgWalUsage.wal_fpi; OffsetNumber deadoffsets[MaxHeapTuplesPerPage]; @@ -1580,10 +1610,15 @@ lazy_scan_prune(LVRelState *vacrel, * 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.freeze_required || presult.nfrozen == 0 || + do_freeze = pagefrz.freeze_required || (presult.all_visible_except_removable && presult.all_frozen && - fpi_before != pgWalUsage.wal_fpi)) + presult.nfrozen > 0 && + fpi_before != pgWalUsage.wal_fpi); + + if (do_freeze) { + 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. @@ -1591,52 +1626,39 @@ lazy_scan_prune(LVRelState *vacrel, vacrel->NewRelfrozenXid = pagefrz.FreezePageRelfrozenXid; vacrel->NewRelminMxid = pagefrz.FreezePageRelminMxid; - if (presult.nfrozen == 0) - { - /* - * 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). - */ - } - else - { - TransactionId snapshotConflictHorizon; + vacrel->frozen_pages++; - vacrel->frozen_pages++; + snapshotConflictHorizon = heap_frz_conflict_horizon(&presult, &pagefrz); - /* - * We can use frz_conflict_horizon as our cutoff for conflicts - * when the whole page is eligible to become all-frozen in the VM - * once we're done with it. Otherwise we generate a conservative - * cutoff by stepping back from OldestXmin. - */ - if (presult.all_visible_except_removable && presult.all_frozen) - { - /* Using same cutoff when setting VM is now unnecessary */ - snapshotConflictHorizon = presult.frz_conflict_horizon; - presult.frz_conflict_horizon = InvalidTransactionId; - } - else - { - /* Avoids false conflicts when hot_standby_feedback in use */ - snapshotConflictHorizon = vacrel->cutoffs.OldestXmin; - TransactionIdRetreat(snapshotConflictHorizon); - } + /* Using same cutoff when setting VM is now unnecessary */ + if (presult.all_visible_except_removable && 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); - } + /* 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 { -- 2.40.1 --racicctn4wry6xe5 Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v3-0007-Execute-freezing-in-heap_page_prune.patch" ^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH v2 06/17] lazy_scan_prune reorder freeze execution logic @ 2024-01-07 19:50 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 14+ messages in thread From: Melanie Plageman @ 2024-01-07 19:50 UTC (permalink / raw) To combine the prune and freeze records, freezing must be done before a pruning WAL record is emitted. We will move the freeze execution into heap_page_prune() in future commits. lazy_scan_prune() currently executes freezing, updates vacrel->NewRelfrozenXid and vacrel->NewRelminMxid, and resets the snapshotConflictHorizon that the visibility map update record may use in the same block of if statements. This commit starts reordering that logic so that the freeze execution can be separated from the other updates which should not be done in pruning. It also adds a helper calculating freeze snapshot conflict horizon. This will be useful when the freeze execution is moved into pruning because not all callers of heap_page_prune() have access to VacuumCutoffs. --- src/backend/access/heap/vacuumlazy.c | 112 ++++++++++++++++----------- 1 file changed, 67 insertions(+), 45 deletions(-) diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index 4187c998d25..abbb7ab3ada 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -269,6 +269,8 @@ 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 @@ -1373,6 +1375,33 @@ lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf, BlockNumber blkno, return false; } +/* + * 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 result; + + /* + * We can use frz_conflict_horizon as our cutoff for conflicts when the + * whole page is eligible to become all-frozen in the VM once we're done + * with it. Otherwise we generate a conservative cutoff by stepping back + * from OldestXmin. + */ + if (presult->all_visible_except_removable && presult->all_frozen) + result = presult->frz_conflict_horizon; + else + { + /* Avoids false conflicts when hot_standby_feedback in use */ + result = pagefrz->cutoffs->OldestXmin; + TransactionIdRetreat(result); + } + + return result; +} + /* * lazy_scan_prune() -- lazy_scan_heap() pruning and freezing. * @@ -1421,6 +1450,7 @@ lazy_scan_prune(LVRelState *vacrel, recently_dead_tuples; HeapPageFreeze pagefrz; bool hastup = false; + bool do_freeze; int64 fpi_before = pgWalUsage.wal_fpi; OffsetNumber deadoffsets[MaxHeapTuplesPerPage]; @@ -1580,10 +1610,15 @@ lazy_scan_prune(LVRelState *vacrel, * 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.freeze_required || presult.nfrozen == 0 || + do_freeze = pagefrz.freeze_required || (presult.all_visible_except_removable && presult.all_frozen && - fpi_before != pgWalUsage.wal_fpi)) + presult.nfrozen > 0 && + fpi_before != pgWalUsage.wal_fpi); + + if (do_freeze) { + 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. @@ -1591,52 +1626,39 @@ lazy_scan_prune(LVRelState *vacrel, vacrel->NewRelfrozenXid = pagefrz.FreezePageRelfrozenXid; vacrel->NewRelminMxid = pagefrz.FreezePageRelminMxid; - if (presult.nfrozen == 0) - { - /* - * 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). - */ - } - else - { - TransactionId snapshotConflictHorizon; + vacrel->frozen_pages++; - vacrel->frozen_pages++; + snapshotConflictHorizon = heap_frz_conflict_horizon(&presult, &pagefrz); - /* - * We can use frz_conflict_horizon as our cutoff for conflicts - * when the whole page is eligible to become all-frozen in the VM - * once we're done with it. Otherwise we generate a conservative - * cutoff by stepping back from OldestXmin. - */ - if (presult.all_visible_except_removable && presult.all_frozen) - { - /* Using same cutoff when setting VM is now unnecessary */ - snapshotConflictHorizon = presult.frz_conflict_horizon; - presult.frz_conflict_horizon = InvalidTransactionId; - } - else - { - /* Avoids false conflicts when hot_standby_feedback in use */ - snapshotConflictHorizon = vacrel->cutoffs.OldestXmin; - TransactionIdRetreat(snapshotConflictHorizon); - } + /* Using same cutoff when setting VM is now unnecessary */ + if (presult.all_visible_except_removable && 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); - } + /* 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 { -- 2.40.1 --rdqtp5puvxqotfdw Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v2-0007-Execute-freezing-in-heap_page_prune.patch" ^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH v2 06/17] lazy_scan_prune reorder freeze execution logic @ 2024-01-07 19:50 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 14+ messages in thread From: Melanie Plageman @ 2024-01-07 19:50 UTC (permalink / raw) To combine the prune and freeze records, freezing must be done before a pruning WAL record is emitted. We will move the freeze execution into heap_page_prune() in future commits. lazy_scan_prune() currently executes freezing, updates vacrel->NewRelfrozenXid and vacrel->NewRelminMxid, and resets the snapshotConflictHorizon that the visibility map update record may use in the same block of if statements. This commit starts reordering that logic so that the freeze execution can be separated from the other updates which should not be done in pruning. It also adds a helper calculating freeze snapshot conflict horizon. This will be useful when the freeze execution is moved into pruning because not all callers of heap_page_prune() have access to VacuumCutoffs. --- src/backend/access/heap/vacuumlazy.c | 112 ++++++++++++++++----------- 1 file changed, 67 insertions(+), 45 deletions(-) diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index 4187c998d25..abbb7ab3ada 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -269,6 +269,8 @@ 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 @@ -1373,6 +1375,33 @@ lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf, BlockNumber blkno, return false; } +/* + * 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 result; + + /* + * We can use frz_conflict_horizon as our cutoff for conflicts when the + * whole page is eligible to become all-frozen in the VM once we're done + * with it. Otherwise we generate a conservative cutoff by stepping back + * from OldestXmin. + */ + if (presult->all_visible_except_removable && presult->all_frozen) + result = presult->frz_conflict_horizon; + else + { + /* Avoids false conflicts when hot_standby_feedback in use */ + result = pagefrz->cutoffs->OldestXmin; + TransactionIdRetreat(result); + } + + return result; +} + /* * lazy_scan_prune() -- lazy_scan_heap() pruning and freezing. * @@ -1421,6 +1450,7 @@ lazy_scan_prune(LVRelState *vacrel, recently_dead_tuples; HeapPageFreeze pagefrz; bool hastup = false; + bool do_freeze; int64 fpi_before = pgWalUsage.wal_fpi; OffsetNumber deadoffsets[MaxHeapTuplesPerPage]; @@ -1580,10 +1610,15 @@ lazy_scan_prune(LVRelState *vacrel, * 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.freeze_required || presult.nfrozen == 0 || + do_freeze = pagefrz.freeze_required || (presult.all_visible_except_removable && presult.all_frozen && - fpi_before != pgWalUsage.wal_fpi)) + presult.nfrozen > 0 && + fpi_before != pgWalUsage.wal_fpi); + + if (do_freeze) { + 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. @@ -1591,52 +1626,39 @@ lazy_scan_prune(LVRelState *vacrel, vacrel->NewRelfrozenXid = pagefrz.FreezePageRelfrozenXid; vacrel->NewRelminMxid = pagefrz.FreezePageRelminMxid; - if (presult.nfrozen == 0) - { - /* - * 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). - */ - } - else - { - TransactionId snapshotConflictHorizon; + vacrel->frozen_pages++; - vacrel->frozen_pages++; + snapshotConflictHorizon = heap_frz_conflict_horizon(&presult, &pagefrz); - /* - * We can use frz_conflict_horizon as our cutoff for conflicts - * when the whole page is eligible to become all-frozen in the VM - * once we're done with it. Otherwise we generate a conservative - * cutoff by stepping back from OldestXmin. - */ - if (presult.all_visible_except_removable && presult.all_frozen) - { - /* Using same cutoff when setting VM is now unnecessary */ - snapshotConflictHorizon = presult.frz_conflict_horizon; - presult.frz_conflict_horizon = InvalidTransactionId; - } - else - { - /* Avoids false conflicts when hot_standby_feedback in use */ - snapshotConflictHorizon = vacrel->cutoffs.OldestXmin; - TransactionIdRetreat(snapshotConflictHorizon); - } + /* Using same cutoff when setting VM is now unnecessary */ + if (presult.all_visible_except_removable && 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); - } + /* 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 { -- 2.40.1 --rdqtp5puvxqotfdw Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v2-0007-Execute-freezing-in-heap_page_prune.patch" ^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH v3 06/17] lazy_scan_prune reorder freeze execution logic @ 2024-01-07 19:50 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 14+ messages in thread From: Melanie Plageman @ 2024-01-07 19:50 UTC (permalink / raw) To combine the prune and freeze records, freezing must be done before a pruning WAL record is emitted. We will move the freeze execution into heap_page_prune() in future commits. lazy_scan_prune() currently executes freezing, updates vacrel->NewRelfrozenXid and vacrel->NewRelminMxid, and resets the snapshotConflictHorizon that the visibility map update record may use in the same block of if statements. This commit starts reordering that logic so that the freeze execution can be separated from the other updates which should not be done in pruning. It also adds a helper calculating freeze snapshot conflict horizon. This will be useful when the freeze execution is moved into pruning because not all callers of heap_page_prune() have access to VacuumCutoffs. --- src/backend/access/heap/vacuumlazy.c | 112 ++++++++++++++++----------- 1 file changed, 67 insertions(+), 45 deletions(-) diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index 4187c998d25..abbb7ab3ada 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -269,6 +269,8 @@ 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 @@ -1373,6 +1375,33 @@ lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf, BlockNumber blkno, return false; } +/* + * 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 result; + + /* + * We can use frz_conflict_horizon as our cutoff for conflicts when the + * whole page is eligible to become all-frozen in the VM once we're done + * with it. Otherwise we generate a conservative cutoff by stepping back + * from OldestXmin. + */ + if (presult->all_visible_except_removable && presult->all_frozen) + result = presult->frz_conflict_horizon; + else + { + /* Avoids false conflicts when hot_standby_feedback in use */ + result = pagefrz->cutoffs->OldestXmin; + TransactionIdRetreat(result); + } + + return result; +} + /* * lazy_scan_prune() -- lazy_scan_heap() pruning and freezing. * @@ -1421,6 +1450,7 @@ lazy_scan_prune(LVRelState *vacrel, recently_dead_tuples; HeapPageFreeze pagefrz; bool hastup = false; + bool do_freeze; int64 fpi_before = pgWalUsage.wal_fpi; OffsetNumber deadoffsets[MaxHeapTuplesPerPage]; @@ -1580,10 +1610,15 @@ lazy_scan_prune(LVRelState *vacrel, * 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.freeze_required || presult.nfrozen == 0 || + do_freeze = pagefrz.freeze_required || (presult.all_visible_except_removable && presult.all_frozen && - fpi_before != pgWalUsage.wal_fpi)) + presult.nfrozen > 0 && + fpi_before != pgWalUsage.wal_fpi); + + if (do_freeze) { + 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. @@ -1591,52 +1626,39 @@ lazy_scan_prune(LVRelState *vacrel, vacrel->NewRelfrozenXid = pagefrz.FreezePageRelfrozenXid; vacrel->NewRelminMxid = pagefrz.FreezePageRelminMxid; - if (presult.nfrozen == 0) - { - /* - * 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). - */ - } - else - { - TransactionId snapshotConflictHorizon; + vacrel->frozen_pages++; - vacrel->frozen_pages++; + snapshotConflictHorizon = heap_frz_conflict_horizon(&presult, &pagefrz); - /* - * We can use frz_conflict_horizon as our cutoff for conflicts - * when the whole page is eligible to become all-frozen in the VM - * once we're done with it. Otherwise we generate a conservative - * cutoff by stepping back from OldestXmin. - */ - if (presult.all_visible_except_removable && presult.all_frozen) - { - /* Using same cutoff when setting VM is now unnecessary */ - snapshotConflictHorizon = presult.frz_conflict_horizon; - presult.frz_conflict_horizon = InvalidTransactionId; - } - else - { - /* Avoids false conflicts when hot_standby_feedback in use */ - snapshotConflictHorizon = vacrel->cutoffs.OldestXmin; - TransactionIdRetreat(snapshotConflictHorizon); - } + /* Using same cutoff when setting VM is now unnecessary */ + if (presult.all_visible_except_removable && 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); - } + /* 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 { -- 2.40.1 --racicctn4wry6xe5 Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v3-0007-Execute-freezing-in-heap_page_prune.patch" ^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH v4 08/19] lazy_scan_prune reorder freeze execution logic @ 2024-03-19 23:30 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 14+ messages in thread From: Melanie Plageman @ 2024-03-19 23:30 UTC (permalink / raw) To combine the prune and freeze records, freezing must be done before a pruning WAL record is emitted. We will move the freeze execution into heap_page_prune() in future commits. lazy_scan_prune() currently executes freezing, updates vacrel->NewRelfrozenXid and vacrel->NewRelminMxid, and resets the snapshotConflictHorizon that the visibility map update record may use in the same block of if statements. This commit starts reordering that logic so that the freeze execution can be separated from the other updates which should not be done in pruning. --- src/backend/access/heap/vacuumlazy.c | 92 +++++++++++++++------------- 1 file changed, 49 insertions(+), 43 deletions(-) diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index 4187c998d25..74ebab25a95 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -1421,6 +1421,7 @@ lazy_scan_prune(LVRelState *vacrel, recently_dead_tuples; HeapPageFreeze pagefrz; bool hastup = false; + bool do_freeze; int64 fpi_before = pgWalUsage.wal_fpi; OffsetNumber deadoffsets[MaxHeapTuplesPerPage]; @@ -1580,10 +1581,15 @@ lazy_scan_prune(LVRelState *vacrel, * 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.freeze_required || presult.nfrozen == 0 || + do_freeze = pagefrz.freeze_required || (presult.all_visible_except_removable && presult.all_frozen && - fpi_before != pgWalUsage.wal_fpi)) + presult.nfrozen > 0 && + fpi_before != pgWalUsage.wal_fpi); + + if (do_freeze) { + 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. @@ -1591,52 +1597,52 @@ lazy_scan_prune(LVRelState *vacrel, vacrel->NewRelfrozenXid = pagefrz.FreezePageRelfrozenXid; vacrel->NewRelminMxid = pagefrz.FreezePageRelminMxid; - if (presult.nfrozen == 0) - { - /* - * 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->frozen_pages++; + + /* + * We can use frz_conflict_horizon as our cutoff for conflicts when + * the whole page is eligible to become all-frozen in the VM once + * we're done with it. Otherwise we generate a conservative cutoff by + * stepping back from OldestXmin. + */ + if (presult.all_visible_except_removable && presult.all_frozen) + snapshotConflictHorizon = presult.frz_conflict_horizon; else { - TransactionId snapshotConflictHorizon; + /* Avoids false conflicts when hot_standby_feedback in use */ + snapshotConflictHorizon = pagefrz.cutoffs->OldestXmin; + TransactionIdRetreat(snapshotConflictHorizon); + } - vacrel->frozen_pages++; + /* Using same cutoff when setting VM is now unnecessary */ + if (presult.all_visible_except_removable && presult.all_frozen) + presult.frz_conflict_horizon = InvalidTransactionId; - /* - * We can use frz_conflict_horizon as our cutoff for conflicts - * when the whole page is eligible to become all-frozen in the VM - * once we're done with it. Otherwise we generate a conservative - * cutoff by stepping back from OldestXmin. - */ - if (presult.all_visible_except_removable && presult.all_frozen) - { - /* Using same cutoff when setting VM is now unnecessary */ - snapshotConflictHorizon = presult.frz_conflict_horizon; - presult.frz_conflict_horizon = InvalidTransactionId; - } - else - { - /* Avoids false conflicts when hot_standby_feedback in use */ - snapshotConflictHorizon = vacrel->cutoffs.OldestXmin; - TransactionIdRetreat(snapshotConflictHorizon); - } + /* 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); - /* Execute all freeze plans for page as a single atomic action */ - heap_freeze_execute_prepared(vacrel->rel, buf, - snapshotConflictHorizon, - presult.frozen, presult.nfrozen); - } + /* + * 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 { -- 2.40.1 --tez7m2a73jtztiij Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v4-0009-Execute-freezing-in-heap_page_prune.patch" ^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH v4 08/19] lazy_scan_prune reorder freeze execution logic @ 2024-03-19 23:30 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 14+ messages in thread From: Melanie Plageman @ 2024-03-19 23:30 UTC (permalink / raw) To combine the prune and freeze records, freezing must be done before a pruning WAL record is emitted. We will move the freeze execution into heap_page_prune() in future commits. lazy_scan_prune() currently executes freezing, updates vacrel->NewRelfrozenXid and vacrel->NewRelminMxid, and resets the snapshotConflictHorizon that the visibility map update record may use in the same block of if statements. This commit starts reordering that logic so that the freeze execution can be separated from the other updates which should not be done in pruning. --- src/backend/access/heap/vacuumlazy.c | 92 +++++++++++++++------------- 1 file changed, 49 insertions(+), 43 deletions(-) diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index 4187c998d25..74ebab25a95 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -1421,6 +1421,7 @@ lazy_scan_prune(LVRelState *vacrel, recently_dead_tuples; HeapPageFreeze pagefrz; bool hastup = false; + bool do_freeze; int64 fpi_before = pgWalUsage.wal_fpi; OffsetNumber deadoffsets[MaxHeapTuplesPerPage]; @@ -1580,10 +1581,15 @@ lazy_scan_prune(LVRelState *vacrel, * 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.freeze_required || presult.nfrozen == 0 || + do_freeze = pagefrz.freeze_required || (presult.all_visible_except_removable && presult.all_frozen && - fpi_before != pgWalUsage.wal_fpi)) + presult.nfrozen > 0 && + fpi_before != pgWalUsage.wal_fpi); + + if (do_freeze) { + 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. @@ -1591,52 +1597,52 @@ lazy_scan_prune(LVRelState *vacrel, vacrel->NewRelfrozenXid = pagefrz.FreezePageRelfrozenXid; vacrel->NewRelminMxid = pagefrz.FreezePageRelminMxid; - if (presult.nfrozen == 0) - { - /* - * 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->frozen_pages++; + + /* + * We can use frz_conflict_horizon as our cutoff for conflicts when + * the whole page is eligible to become all-frozen in the VM once + * we're done with it. Otherwise we generate a conservative cutoff by + * stepping back from OldestXmin. + */ + if (presult.all_visible_except_removable && presult.all_frozen) + snapshotConflictHorizon = presult.frz_conflict_horizon; else { - TransactionId snapshotConflictHorizon; + /* Avoids false conflicts when hot_standby_feedback in use */ + snapshotConflictHorizon = pagefrz.cutoffs->OldestXmin; + TransactionIdRetreat(snapshotConflictHorizon); + } - vacrel->frozen_pages++; + /* Using same cutoff when setting VM is now unnecessary */ + if (presult.all_visible_except_removable && presult.all_frozen) + presult.frz_conflict_horizon = InvalidTransactionId; - /* - * We can use frz_conflict_horizon as our cutoff for conflicts - * when the whole page is eligible to become all-frozen in the VM - * once we're done with it. Otherwise we generate a conservative - * cutoff by stepping back from OldestXmin. - */ - if (presult.all_visible_except_removable && presult.all_frozen) - { - /* Using same cutoff when setting VM is now unnecessary */ - snapshotConflictHorizon = presult.frz_conflict_horizon; - presult.frz_conflict_horizon = InvalidTransactionId; - } - else - { - /* Avoids false conflicts when hot_standby_feedback in use */ - snapshotConflictHorizon = vacrel->cutoffs.OldestXmin; - TransactionIdRetreat(snapshotConflictHorizon); - } + /* 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); - /* Execute all freeze plans for page as a single atomic action */ - heap_freeze_execute_prepared(vacrel->rel, buf, - snapshotConflictHorizon, - presult.frozen, presult.nfrozen); - } + /* + * 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 { -- 2.40.1 --tez7m2a73jtztiij Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v4-0009-Execute-freezing-in-heap_page_prune.patch" ^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH v4 08/19] lazy_scan_prune reorder freeze execution logic @ 2024-03-19 23:30 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 14+ messages in thread From: Melanie Plageman @ 2024-03-19 23:30 UTC (permalink / raw) To combine the prune and freeze records, freezing must be done before a pruning WAL record is emitted. We will move the freeze execution into heap_page_prune() in future commits. lazy_scan_prune() currently executes freezing, updates vacrel->NewRelfrozenXid and vacrel->NewRelminMxid, and resets the snapshotConflictHorizon that the visibility map update record may use in the same block of if statements. This commit starts reordering that logic so that the freeze execution can be separated from the other updates which should not be done in pruning. --- src/backend/access/heap/vacuumlazy.c | 92 +++++++++++++++------------- 1 file changed, 49 insertions(+), 43 deletions(-) diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index 4187c998d25..74ebab25a95 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -1421,6 +1421,7 @@ lazy_scan_prune(LVRelState *vacrel, recently_dead_tuples; HeapPageFreeze pagefrz; bool hastup = false; + bool do_freeze; int64 fpi_before = pgWalUsage.wal_fpi; OffsetNumber deadoffsets[MaxHeapTuplesPerPage]; @@ -1580,10 +1581,15 @@ lazy_scan_prune(LVRelState *vacrel, * 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.freeze_required || presult.nfrozen == 0 || + do_freeze = pagefrz.freeze_required || (presult.all_visible_except_removable && presult.all_frozen && - fpi_before != pgWalUsage.wal_fpi)) + presult.nfrozen > 0 && + fpi_before != pgWalUsage.wal_fpi); + + if (do_freeze) { + 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. @@ -1591,52 +1597,52 @@ lazy_scan_prune(LVRelState *vacrel, vacrel->NewRelfrozenXid = pagefrz.FreezePageRelfrozenXid; vacrel->NewRelminMxid = pagefrz.FreezePageRelminMxid; - if (presult.nfrozen == 0) - { - /* - * 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->frozen_pages++; + + /* + * We can use frz_conflict_horizon as our cutoff for conflicts when + * the whole page is eligible to become all-frozen in the VM once + * we're done with it. Otherwise we generate a conservative cutoff by + * stepping back from OldestXmin. + */ + if (presult.all_visible_except_removable && presult.all_frozen) + snapshotConflictHorizon = presult.frz_conflict_horizon; else { - TransactionId snapshotConflictHorizon; + /* Avoids false conflicts when hot_standby_feedback in use */ + snapshotConflictHorizon = pagefrz.cutoffs->OldestXmin; + TransactionIdRetreat(snapshotConflictHorizon); + } - vacrel->frozen_pages++; + /* Using same cutoff when setting VM is now unnecessary */ + if (presult.all_visible_except_removable && presult.all_frozen) + presult.frz_conflict_horizon = InvalidTransactionId; - /* - * We can use frz_conflict_horizon as our cutoff for conflicts - * when the whole page is eligible to become all-frozen in the VM - * once we're done with it. Otherwise we generate a conservative - * cutoff by stepping back from OldestXmin. - */ - if (presult.all_visible_except_removable && presult.all_frozen) - { - /* Using same cutoff when setting VM is now unnecessary */ - snapshotConflictHorizon = presult.frz_conflict_horizon; - presult.frz_conflict_horizon = InvalidTransactionId; - } - else - { - /* Avoids false conflicts when hot_standby_feedback in use */ - snapshotConflictHorizon = vacrel->cutoffs.OldestXmin; - TransactionIdRetreat(snapshotConflictHorizon); - } + /* 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); - /* Execute all freeze plans for page as a single atomic action */ - heap_freeze_execute_prepared(vacrel->rel, buf, - snapshotConflictHorizon, - presult.frozen, presult.nfrozen); - } + /* + * 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 { -- 2.40.1 --tez7m2a73jtztiij Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v4-0009-Execute-freezing-in-heap_page_prune.patch" ^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH v9 07/21] lazy_scan_prune reorder freeze execution logic @ 2024-03-25 23:39 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 14+ messages in thread From: Melanie Plageman @ 2024-03-25 23:39 UTC (permalink / raw) To combine the prune and freeze records, freezing must be done before a pruning WAL record is emitted. We will move the freeze execution into heap_page_prune() in future commits. lazy_scan_prune() currently executes freezing, updates vacrel->NewRelfrozenXid and vacrel->NewRelminMxid, and resets the snapshotConflictHorizon that the visibility map update record may use in the same block of if statements. This commit starts reordering that logic so that the freeze execution can be separated from the other updates which should not be done in pruning. --- src/backend/access/heap/vacuumlazy.c | 93 +++++++++++++++------------- 1 file changed, 50 insertions(+), 43 deletions(-) diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index 2a3cc5c7cd3..f474e661428 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -1421,6 +1421,7 @@ lazy_scan_prune(LVRelState *vacrel, recently_dead_tuples; HeapPageFreeze pagefrz; bool hastup = false; + bool do_freeze; int64 fpi_before = pgWalUsage.wal_fpi; OffsetNumber deadoffsets[MaxHeapTuplesPerPage]; @@ -1576,10 +1577,15 @@ lazy_scan_prune(LVRelState *vacrel, * 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.freeze_required || presult.nfrozen == 0 || + do_freeze = pagefrz.freeze_required || (presult.all_visible_except_removable && presult.all_frozen && - fpi_before != pgWalUsage.wal_fpi)) + presult.nfrozen > 0 && + fpi_before != pgWalUsage.wal_fpi); + + if (do_freeze) { + 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. @@ -1587,52 +1593,53 @@ lazy_scan_prune(LVRelState *vacrel, vacrel->NewRelfrozenXid = pagefrz.FreezePageRelfrozenXid; vacrel->NewRelminMxid = pagefrz.FreezePageRelminMxid; - if (presult.nfrozen == 0) - { - /* - * 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->frozen_pages++; + + /* + * We can use frz_conflict_horizon as our cutoff for conflicts when + * the whole page is eligible to become all-frozen in the VM once + * we're done with it. Otherwise we generate a conservative cutoff by + * stepping back from OldestXmin. + */ + if (presult.all_visible_except_removable && presult.all_frozen) + snapshotConflictHorizon = presult.visibility_cutoff_xid; else { - TransactionId snapshotConflictHorizon; + /* Avoids false conflicts when hot_standby_feedback in use */ + snapshotConflictHorizon = pagefrz.cutoffs->OldestXmin; + TransactionIdRetreat(snapshotConflictHorizon); + } - vacrel->frozen_pages++; + /* Using same cutoff when setting VM is now unnecessary */ + if (presult.all_visible_except_removable && presult.all_frozen) + presult.visibility_cutoff_xid = InvalidTransactionId; - /* - * We can use visibility_cutoff_xid as our cutoff for conflicts - * when the whole page is eligible to become all-frozen in the VM - * once we're done with it. Otherwise we generate a conservative - * cutoff by stepping back from OldestXmin. - */ - if (presult.all_visible_except_removable && presult.all_frozen) - { - /* Using same cutoff when setting VM is now unnecessary */ - snapshotConflictHorizon = presult.visibility_cutoff_xid; - presult.visibility_cutoff_xid = InvalidTransactionId; - } - else - { - /* Avoids false conflicts when hot_standby_feedback in use */ - snapshotConflictHorizon = vacrel->cutoffs.OldestXmin; - TransactionIdRetreat(snapshotConflictHorizon); - } + /* Execute all freeze plans for page as a single atomic action */ + heap_freeze_execute_prepared(vacrel->rel, buf, + snapshotConflictHorizon, + presult.frozen, presult.nfrozen); - /* 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 { -- 2.40.1 --caj67xgx3lukmr5f Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v9-0008-Execute-freezing-in-heap_page_prune.patch" ^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH v9 07/21] lazy_scan_prune reorder freeze execution logic @ 2024-03-25 23:39 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 14+ messages in thread From: Melanie Plageman @ 2024-03-25 23:39 UTC (permalink / raw) To combine the prune and freeze records, freezing must be done before a pruning WAL record is emitted. We will move the freeze execution into heap_page_prune() in future commits. lazy_scan_prune() currently executes freezing, updates vacrel->NewRelfrozenXid and vacrel->NewRelminMxid, and resets the snapshotConflictHorizon that the visibility map update record may use in the same block of if statements. This commit starts reordering that logic so that the freeze execution can be separated from the other updates which should not be done in pruning. --- src/backend/access/heap/vacuumlazy.c | 93 +++++++++++++++------------- 1 file changed, 50 insertions(+), 43 deletions(-) diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index 2a3cc5c7cd3..f474e661428 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -1421,6 +1421,7 @@ lazy_scan_prune(LVRelState *vacrel, recently_dead_tuples; HeapPageFreeze pagefrz; bool hastup = false; + bool do_freeze; int64 fpi_before = pgWalUsage.wal_fpi; OffsetNumber deadoffsets[MaxHeapTuplesPerPage]; @@ -1576,10 +1577,15 @@ lazy_scan_prune(LVRelState *vacrel, * 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.freeze_required || presult.nfrozen == 0 || + do_freeze = pagefrz.freeze_required || (presult.all_visible_except_removable && presult.all_frozen && - fpi_before != pgWalUsage.wal_fpi)) + presult.nfrozen > 0 && + fpi_before != pgWalUsage.wal_fpi); + + if (do_freeze) { + 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. @@ -1587,52 +1593,53 @@ lazy_scan_prune(LVRelState *vacrel, vacrel->NewRelfrozenXid = pagefrz.FreezePageRelfrozenXid; vacrel->NewRelminMxid = pagefrz.FreezePageRelminMxid; - if (presult.nfrozen == 0) - { - /* - * 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->frozen_pages++; + + /* + * We can use frz_conflict_horizon as our cutoff for conflicts when + * the whole page is eligible to become all-frozen in the VM once + * we're done with it. Otherwise we generate a conservative cutoff by + * stepping back from OldestXmin. + */ + if (presult.all_visible_except_removable && presult.all_frozen) + snapshotConflictHorizon = presult.visibility_cutoff_xid; else { - TransactionId snapshotConflictHorizon; + /* Avoids false conflicts when hot_standby_feedback in use */ + snapshotConflictHorizon = pagefrz.cutoffs->OldestXmin; + TransactionIdRetreat(snapshotConflictHorizon); + } - vacrel->frozen_pages++; + /* Using same cutoff when setting VM is now unnecessary */ + if (presult.all_visible_except_removable && presult.all_frozen) + presult.visibility_cutoff_xid = InvalidTransactionId; - /* - * We can use visibility_cutoff_xid as our cutoff for conflicts - * when the whole page is eligible to become all-frozen in the VM - * once we're done with it. Otherwise we generate a conservative - * cutoff by stepping back from OldestXmin. - */ - if (presult.all_visible_except_removable && presult.all_frozen) - { - /* Using same cutoff when setting VM is now unnecessary */ - snapshotConflictHorizon = presult.visibility_cutoff_xid; - presult.visibility_cutoff_xid = InvalidTransactionId; - } - else - { - /* Avoids false conflicts when hot_standby_feedback in use */ - snapshotConflictHorizon = vacrel->cutoffs.OldestXmin; - TransactionIdRetreat(snapshotConflictHorizon); - } + /* Execute all freeze plans for page as a single atomic action */ + heap_freeze_execute_prepared(vacrel->rel, buf, + snapshotConflictHorizon, + presult.frozen, presult.nfrozen); - /* 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 { -- 2.40.1 --caj67xgx3lukmr5f Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v9-0008-Execute-freezing-in-heap_page_prune.patch" ^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH v7 07/16] lazy_scan_prune reorder freeze execution logic @ 2024-03-25 23:39 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 14+ messages in thread From: Melanie Plageman @ 2024-03-25 23:39 UTC (permalink / raw) To combine the prune and freeze records, freezing must be done before a pruning WAL record is emitted. We will move the freeze execution into heap_page_prune() in future commits. lazy_scan_prune() currently executes freezing, updates vacrel->NewRelfrozenXid and vacrel->NewRelminMxid, and resets the snapshotConflictHorizon that the visibility map update record may use in the same block of if statements. This commit starts reordering that logic so that the freeze execution can be separated from the other updates which should not be done in pruning. --- src/backend/access/heap/vacuumlazy.c | 93 +++++++++++++++------------- 1 file changed, 50 insertions(+), 43 deletions(-) diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index 2a3cc5c7cd3..f474e661428 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -1421,6 +1421,7 @@ lazy_scan_prune(LVRelState *vacrel, recently_dead_tuples; HeapPageFreeze pagefrz; bool hastup = false; + bool do_freeze; int64 fpi_before = pgWalUsage.wal_fpi; OffsetNumber deadoffsets[MaxHeapTuplesPerPage]; @@ -1576,10 +1577,15 @@ lazy_scan_prune(LVRelState *vacrel, * 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.freeze_required || presult.nfrozen == 0 || + do_freeze = pagefrz.freeze_required || (presult.all_visible_except_removable && presult.all_frozen && - fpi_before != pgWalUsage.wal_fpi)) + presult.nfrozen > 0 && + fpi_before != pgWalUsage.wal_fpi); + + if (do_freeze) { + 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. @@ -1587,52 +1593,53 @@ lazy_scan_prune(LVRelState *vacrel, vacrel->NewRelfrozenXid = pagefrz.FreezePageRelfrozenXid; vacrel->NewRelminMxid = pagefrz.FreezePageRelminMxid; - if (presult.nfrozen == 0) - { - /* - * 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->frozen_pages++; + + /* + * We can use frz_conflict_horizon as our cutoff for conflicts when + * the whole page is eligible to become all-frozen in the VM once + * we're done with it. Otherwise we generate a conservative cutoff by + * stepping back from OldestXmin. + */ + if (presult.all_visible_except_removable && presult.all_frozen) + snapshotConflictHorizon = presult.visibility_cutoff_xid; else { - TransactionId snapshotConflictHorizon; + /* Avoids false conflicts when hot_standby_feedback in use */ + snapshotConflictHorizon = pagefrz.cutoffs->OldestXmin; + TransactionIdRetreat(snapshotConflictHorizon); + } - vacrel->frozen_pages++; + /* Using same cutoff when setting VM is now unnecessary */ + if (presult.all_visible_except_removable && presult.all_frozen) + presult.visibility_cutoff_xid = InvalidTransactionId; - /* - * We can use visibility_cutoff_xid as our cutoff for conflicts - * when the whole page is eligible to become all-frozen in the VM - * once we're done with it. Otherwise we generate a conservative - * cutoff by stepping back from OldestXmin. - */ - if (presult.all_visible_except_removable && presult.all_frozen) - { - /* Using same cutoff when setting VM is now unnecessary */ - snapshotConflictHorizon = presult.visibility_cutoff_xid; - presult.visibility_cutoff_xid = InvalidTransactionId; - } - else - { - /* Avoids false conflicts when hot_standby_feedback in use */ - snapshotConflictHorizon = vacrel->cutoffs.OldestXmin; - TransactionIdRetreat(snapshotConflictHorizon); - } + /* Execute all freeze plans for page as a single atomic action */ + heap_freeze_execute_prepared(vacrel->rel, buf, + snapshotConflictHorizon, + presult.frozen, presult.nfrozen); - /* 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 { -- 2.40.1 --ck6erxojvlx23byk Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v7-0008-Execute-freezing-in-heap_page_prune.patch" ^ permalink raw reply [nested|flat] 14+ messages in thread
* [PATCH v7 07/16] lazy_scan_prune reorder freeze execution logic @ 2024-03-25 23:39 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 14+ messages in thread From: Melanie Plageman @ 2024-03-25 23:39 UTC (permalink / raw) To combine the prune and freeze records, freezing must be done before a pruning WAL record is emitted. We will move the freeze execution into heap_page_prune() in future commits. lazy_scan_prune() currently executes freezing, updates vacrel->NewRelfrozenXid and vacrel->NewRelminMxid, and resets the snapshotConflictHorizon that the visibility map update record may use in the same block of if statements. This commit starts reordering that logic so that the freeze execution can be separated from the other updates which should not be done in pruning. --- src/backend/access/heap/vacuumlazy.c | 93 +++++++++++++++------------- 1 file changed, 50 insertions(+), 43 deletions(-) diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index 2a3cc5c7cd3..f474e661428 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -1421,6 +1421,7 @@ lazy_scan_prune(LVRelState *vacrel, recently_dead_tuples; HeapPageFreeze pagefrz; bool hastup = false; + bool do_freeze; int64 fpi_before = pgWalUsage.wal_fpi; OffsetNumber deadoffsets[MaxHeapTuplesPerPage]; @@ -1576,10 +1577,15 @@ lazy_scan_prune(LVRelState *vacrel, * 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.freeze_required || presult.nfrozen == 0 || + do_freeze = pagefrz.freeze_required || (presult.all_visible_except_removable && presult.all_frozen && - fpi_before != pgWalUsage.wal_fpi)) + presult.nfrozen > 0 && + fpi_before != pgWalUsage.wal_fpi); + + if (do_freeze) { + 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. @@ -1587,52 +1593,53 @@ lazy_scan_prune(LVRelState *vacrel, vacrel->NewRelfrozenXid = pagefrz.FreezePageRelfrozenXid; vacrel->NewRelminMxid = pagefrz.FreezePageRelminMxid; - if (presult.nfrozen == 0) - { - /* - * 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->frozen_pages++; + + /* + * We can use frz_conflict_horizon as our cutoff for conflicts when + * the whole page is eligible to become all-frozen in the VM once + * we're done with it. Otherwise we generate a conservative cutoff by + * stepping back from OldestXmin. + */ + if (presult.all_visible_except_removable && presult.all_frozen) + snapshotConflictHorizon = presult.visibility_cutoff_xid; else { - TransactionId snapshotConflictHorizon; + /* Avoids false conflicts when hot_standby_feedback in use */ + snapshotConflictHorizon = pagefrz.cutoffs->OldestXmin; + TransactionIdRetreat(snapshotConflictHorizon); + } - vacrel->frozen_pages++; + /* Using same cutoff when setting VM is now unnecessary */ + if (presult.all_visible_except_removable && presult.all_frozen) + presult.visibility_cutoff_xid = InvalidTransactionId; - /* - * We can use visibility_cutoff_xid as our cutoff for conflicts - * when the whole page is eligible to become all-frozen in the VM - * once we're done with it. Otherwise we generate a conservative - * cutoff by stepping back from OldestXmin. - */ - if (presult.all_visible_except_removable && presult.all_frozen) - { - /* Using same cutoff when setting VM is now unnecessary */ - snapshotConflictHorizon = presult.visibility_cutoff_xid; - presult.visibility_cutoff_xid = InvalidTransactionId; - } - else - { - /* Avoids false conflicts when hot_standby_feedback in use */ - snapshotConflictHorizon = vacrel->cutoffs.OldestXmin; - TransactionIdRetreat(snapshotConflictHorizon); - } + /* Execute all freeze plans for page as a single atomic action */ + heap_freeze_execute_prepared(vacrel->rel, buf, + snapshotConflictHorizon, + presult.frozen, presult.nfrozen); - /* 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 { -- 2.40.1 --ck6erxojvlx23byk Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v7-0008-Execute-freezing-in-heap_page_prune.patch" ^ permalink raw reply [nested|flat] 14+ messages in thread
end of thread, other threads:[~2024-03-25 23:39 UTC | newest] Thread overview: 14+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2023-12-02 07:23 Commitfest 2023-11 is almost over John Naylor <[email protected]> 2024-01-07 19:50 [PATCH v2 06/17] lazy_scan_prune reorder freeze execution logic Melanie Plageman <[email protected]> 2024-01-07 19:50 [PATCH v3 06/17] lazy_scan_prune reorder freeze execution logic Melanie Plageman <[email protected]> 2024-01-07 19:50 [PATCH v3 06/17] lazy_scan_prune reorder freeze execution logic Melanie Plageman <[email protected]> 2024-01-07 19:50 [PATCH v2 06/17] lazy_scan_prune reorder freeze execution logic Melanie Plageman <[email protected]> 2024-01-07 19:50 [PATCH v3 06/17] lazy_scan_prune reorder freeze execution logic Melanie Plageman <[email protected]> 2024-01-07 19:50 [PATCH v2 06/17] lazy_scan_prune reorder freeze execution logic Melanie Plageman <[email protected]> 2024-03-19 23:30 [PATCH v4 08/19] lazy_scan_prune reorder freeze execution logic Melanie Plageman <[email protected]> 2024-03-19 23:30 [PATCH v4 08/19] lazy_scan_prune reorder freeze execution logic Melanie Plageman <[email protected]> 2024-03-19 23:30 [PATCH v4 08/19] lazy_scan_prune reorder freeze execution logic Melanie Plageman <[email protected]> 2024-03-25 23:39 [PATCH v9 07/21] lazy_scan_prune reorder freeze execution logic Melanie Plageman <[email protected]> 2024-03-25 23:39 [PATCH v7 07/16] lazy_scan_prune reorder freeze execution logic Melanie Plageman <[email protected]> 2024-03-25 23:39 [PATCH v9 07/21] lazy_scan_prune reorder freeze execution logic Melanie Plageman <[email protected]> 2024-03-25 23:39 [PATCH v7 07/16] lazy_scan_prune reorder freeze execution logic Melanie Plageman <[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