Skip to content

Commit

Permalink
Revert most of 39b66a9
Browse files Browse the repository at this point in the history
Reverts most of commit 39b66a9, which was found to cause significant
regression for REFRESH MATERIALIZED VIEW. This means only rows inserted
by heap_multi_insert will benefit from the optimization, implemented in
commit 7db0cd2.

Reported-by: Masahiko Sawada
Discussion: https://postgr.es/m/CAD21AoA%3D%3Df2VSw3c-Cp_y%3DWLKHMKc1D6s7g3YWsCOvgaYPpJcg%40mail.gmail.com
  • Loading branch information
tvondra committed Jun 2, 2021
1 parent 8895923 commit 8e03eb9
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 84 deletions.
74 changes: 1 addition & 73 deletions src/backend/access/heap/heapam.c
Expand Up @@ -2063,12 +2063,8 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
TransactionId xid = GetCurrentTransactionId();
HeapTuple heaptup;
Buffer buffer;
Page page = NULL;
Buffer vmbuffer = InvalidBuffer;
bool starting_with_empty_page;
bool all_visible_cleared = false;
bool all_frozen_set = false;
uint8 vmstatus = 0;

/* Cheap, simplistic check that the tuple matches the rel's rowtype. */
Assert(HeapTupleHeaderGetNatts(tup->t_data) <=
Expand All @@ -2085,36 +2081,11 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
/*
* Find buffer to insert this tuple into. If the page is all visible,
* this will also pin the requisite visibility map page.
*
* Also pin visibility map page if COPY FREEZE inserts tuples into an
* empty page. See all_frozen_set below.
*/
buffer = RelationGetBufferForTuple(relation, heaptup->t_len,
InvalidBuffer, options, bistate,
&vmbuffer, NULL);


/*
* If we're inserting frozen entry into an empty page, set visibility map
* bits and PageAllVisible() hint.
*
* If we're inserting frozen entry into already all_frozen page, preserve
* this state.
*/
if (options & HEAP_INSERT_FROZEN)
{
page = BufferGetPage(buffer);

starting_with_empty_page = PageGetMaxOffsetNumber(page) == 0;

if (visibilitymap_pin_ok(BufferGetBlockNumber(buffer), vmbuffer))
vmstatus = visibilitymap_get_status(relation,
BufferGetBlockNumber(buffer), &vmbuffer);

if ((starting_with_empty_page || vmstatus & VISIBILITYMAP_ALL_FROZEN))
all_frozen_set = true;
}

/*
* We're about to do the actual insert -- but check for conflict first, to
* avoid possibly having to roll back work we've just done.
Expand All @@ -2138,28 +2109,14 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
RelationPutHeapTuple(relation, buffer, heaptup,
(options & HEAP_INSERT_SPECULATIVE) != 0);

/*
* If the page is all visible, need to clear that, unless we're only going
* to add further frozen rows to it.
*
* If we're only adding already frozen rows to a page that was empty or
* marked as all visible, mark it as all-visible.
*/
if (PageIsAllVisible(BufferGetPage(buffer)) && !(options & HEAP_INSERT_FROZEN))
if (PageIsAllVisible(BufferGetPage(buffer)))
{
all_visible_cleared = true;
PageClearAllVisible(BufferGetPage(buffer));
visibilitymap_clear(relation,
ItemPointerGetBlockNumber(&(heaptup->t_self)),
vmbuffer, VISIBILITYMAP_VALID_BITS);
}
else if (all_frozen_set)
{
/* We only ever set all_frozen_set after reading the page. */
Assert(page);

PageSetAllVisible(page);
}

/*
* XXX Should we set PageSetPrunable on this page ?
Expand Down Expand Up @@ -2207,8 +2164,6 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
xlrec.flags = 0;
if (all_visible_cleared)
xlrec.flags |= XLH_INSERT_ALL_VISIBLE_CLEARED;
if (all_frozen_set)
xlrec.flags = XLH_INSERT_ALL_FROZEN_SET;
if (options & HEAP_INSERT_SPECULATIVE)
xlrec.flags |= XLH_INSERT_IS_SPECULATIVE;
Assert(ItemPointerGetBlockNumber(&heaptup->t_self) == BufferGetBlockNumber(buffer));
Expand Down Expand Up @@ -2257,29 +2212,6 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,

END_CRIT_SECTION();

/*
* If we've frozen everything on the page, update the visibilitymap. We're
* already holding pin on the vmbuffer.
*
* No need to update the visibilitymap if it had all_frozen bit set before
* this insertion.
*/
if (all_frozen_set && ((vmstatus & VISIBILITYMAP_ALL_FROZEN) == 0))
{
Assert(PageIsAllVisible(page));
Assert(visibilitymap_pin_ok(BufferGetBlockNumber(buffer), vmbuffer));

/*
* It's fine to use InvalidTransactionId here - this is only used when
* HEAP_INSERT_FROZEN is specified, which intentionally violates
* visibility rules.
*/
visibilitymap_set(relation, BufferGetBlockNumber(buffer), buffer,
InvalidXLogRecPtr, vmbuffer,
InvalidTransactionId,
VISIBILITYMAP_ALL_VISIBLE | VISIBILITYMAP_ALL_FROZEN);
}

UnlockReleaseBuffer(buffer);
if (vmbuffer != InvalidBuffer)
ReleaseBuffer(vmbuffer);
Expand Down Expand Up @@ -8946,10 +8878,6 @@ heap_xlog_insert(XLogReaderState *record)
ItemPointerSetBlockNumber(&target_tid, blkno);
ItemPointerSetOffsetNumber(&target_tid, xlrec->offnum);

/* check that the mutually exclusive flags are not both set */
Assert(!((xlrec->flags & XLH_INSERT_ALL_VISIBLE_CLEARED) &&
(xlrec->flags & XLH_INSERT_ALL_FROZEN_SET)));

/*
* The visibility map may need to be fixed even if the heap page is
* already up-to-date.
Expand Down
22 changes: 11 additions & 11 deletions src/backend/access/heap/hio.c
Expand Up @@ -407,19 +407,19 @@ RelationGetBufferForTuple(Relation relation, Size len,
* target.
*/
targetBlock = GetPageWithFreeSpace(relation, targetFreeSpace);
}

/*
* If the FSM knows nothing of the rel, try the last page before we give
* up and extend. This avoids one-tuple-per-page syndrome during
* bootstrapping or in a recently-started system.
*/
if (targetBlock == InvalidBlockNumber)
{
BlockNumber nblocks = RelationGetNumberOfBlocks(relation);
/*
* If the FSM knows nothing of the rel, try the last page before we
* give up and extend. This avoids one-tuple-per-page syndrome during
* bootstrapping or in a recently-started system.
*/
if (targetBlock == InvalidBlockNumber)
{
BlockNumber nblocks = RelationGetNumberOfBlocks(relation);

if (nblocks > 0)
targetBlock = nblocks - 1;
if (nblocks > 0)
targetBlock = nblocks - 1;
}
}

loop:
Expand Down

0 comments on commit 8e03eb9

Please sign in to comment.