Skip to content

Commit ea13e61

Browse files
melanieplagemanCommitfest Bot
authored andcommitted
Keep all_frozen updated in heap_page_prune_and_freeze
Previously, we relied on all_visible and all_frozen being used together to ensure that all_frozen was correct, but it is better to keep both fields updated. Future changes will separate their usage, so we should not depend on all_visible for the validity of all_frozen.
1 parent 0d23b78 commit ea13e61

File tree

2 files changed

+15
-16
lines changed

2 files changed

+15
-16
lines changed

src/backend/access/heap/pruneheap.c

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -143,10 +143,6 @@ typedef struct
143143
* whether to freeze the page or not. The all_visible and all_frozen
144144
* values returned to the caller are adjusted to include LP_DEAD items at
145145
* the end.
146-
*
147-
* all_frozen should only be considered valid if all_visible is also set;
148-
* we don't bother to clear the all_frozen flag every time we clear the
149-
* all_visible flag.
150146
*/
151147
bool all_visible;
152148
bool all_frozen;
@@ -361,8 +357,10 @@ heap_page_will_freeze(Relation relation, Buffer buffer,
361357
* anymore. The opportunistic freeze heuristic must be improved;
362358
* however, for now, try to approximate the old logic.
363359
*/
364-
if (prstate->all_visible && prstate->all_frozen && prstate->nfrozen > 0)
360+
if (prstate->all_frozen && prstate->nfrozen > 0)
365361
{
362+
Assert(prstate->all_visible);
363+
366364
/*
367365
* Freezing would make the page all-frozen. Have already emitted
368366
* an FPI or will do so anyway?
@@ -784,6 +782,8 @@ heap_page_prune_and_freeze(PruneFreezeParams *params,
784782
do_hint_prune,
785783
&prstate);
786784

785+
Assert(!prstate.all_frozen || prstate.all_visible);
786+
787787
/* Any error while applying the changes is critical */
788788
START_CRIT_SECTION();
789789

@@ -853,7 +853,7 @@ heap_page_prune_and_freeze(PruneFreezeParams *params,
853853
*/
854854
if (do_freeze)
855855
{
856-
if (prstate.all_visible && prstate.all_frozen)
856+
if (prstate.all_frozen)
857857
frz_conflict_horizon = prstate.visibility_cutoff_xid;
858858
else
859859
{
@@ -1418,7 +1418,7 @@ heap_prune_record_unchanged_lp_normal(Page page, PruneState *prstate, OffsetNumb
14181418

14191419
if (!HeapTupleHeaderXminCommitted(htup))
14201420
{
1421-
prstate->all_visible = false;
1421+
prstate->all_visible = prstate->all_frozen = false;
14221422
break;
14231423
}
14241424

@@ -1440,7 +1440,7 @@ heap_prune_record_unchanged_lp_normal(Page page, PruneState *prstate, OffsetNumb
14401440
Assert(prstate->cutoffs);
14411441
if (!TransactionIdPrecedes(xmin, prstate->cutoffs->OldestXmin))
14421442
{
1443-
prstate->all_visible = false;
1443+
prstate->all_visible = prstate->all_frozen = false;
14441444
break;
14451445
}
14461446

@@ -1453,7 +1453,7 @@ heap_prune_record_unchanged_lp_normal(Page page, PruneState *prstate, OffsetNumb
14531453

14541454
case HEAPTUPLE_RECENTLY_DEAD:
14551455
prstate->recently_dead_tuples++;
1456-
prstate->all_visible = false;
1456+
prstate->all_visible = prstate->all_frozen = false;
14571457

14581458
/*
14591459
* This tuple will soon become DEAD. Update the hint field so
@@ -1472,7 +1472,7 @@ heap_prune_record_unchanged_lp_normal(Page page, PruneState *prstate, OffsetNumb
14721472
* assumption is a bit shaky, but it is what acquire_sample_rows()
14731473
* does, so be consistent.
14741474
*/
1475-
prstate->all_visible = false;
1475+
prstate->all_visible = prstate->all_frozen = false;
14761476

14771477
/*
14781478
* If we wanted to optimize for aborts, we might consider marking
@@ -1490,7 +1490,7 @@ heap_prune_record_unchanged_lp_normal(Page page, PruneState *prstate, OffsetNumb
14901490
* will commit and update the counters after we report.
14911491
*/
14921492
prstate->live_tuples++;
1493-
prstate->all_visible = false;
1493+
prstate->all_visible = prstate->all_frozen = false;
14941494

14951495
/*
14961496
* This tuple may soon become DEAD. Update the hint field so that

src/backend/access/heap/vacuumlazy.c

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2020,7 +2020,6 @@ lazy_scan_prune(LVRelState *vacrel,
20202020
* agreement with heap_page_is_all_visible() using an assertion.
20212021
*/
20222022
#ifdef USE_ASSERT_CHECKING
2023-
/* Note that all_frozen value does not matter when !all_visible */
20242023
if (presult.all_visible)
20252024
{
20262025
TransactionId debug_cutoff;
@@ -2074,6 +2073,7 @@ lazy_scan_prune(LVRelState *vacrel,
20742073
*has_lpdead_items = (presult.lpdead_items > 0);
20752074

20762075
Assert(!presult.all_visible || !(*has_lpdead_items));
2076+
Assert(!presult.all_frozen || presult.all_visible);
20772077

20782078
/*
20792079
* Handle setting visibility map bit based on information from the VM (as
@@ -2179,11 +2179,10 @@ lazy_scan_prune(LVRelState *vacrel,
21792179

21802180
/*
21812181
* If the all-visible page is all-frozen but not marked as such yet, mark
2182-
* it as all-frozen. Note that all_frozen is only valid if all_visible is
2183-
* true, so we must check both all_visible and all_frozen.
2182+
* it as all-frozen.
21842183
*/
2185-
else if (all_visible_according_to_vm && presult.all_visible &&
2186-
presult.all_frozen && !VM_ALL_FROZEN(vacrel->rel, blkno, &vmbuffer))
2184+
else if (all_visible_according_to_vm && presult.all_frozen &&
2185+
!VM_ALL_FROZEN(vacrel->rel, blkno, &vmbuffer))
21872186
{
21882187
uint8 old_vmbits;
21892188

0 commit comments

Comments
 (0)