Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 4da99ea

Browse files
committed
Avoid having two copies of the HOT-chain search logic.
It's been like this since HOT was originally introduced, but the logic is complex enough that this is a recipe for bugs, as we've already found out with SSI. So refactor heap_hot_search_buffer() so that it can satisfy the needs of index_getnext(), and make index_getnext() use that rather than duplicating the logic. This change was originally proposed by Heikki Linnakangas as part of a larger refactoring oriented towards allowing index-only scans. I extracted and adjusted this part, since it seems to have independent merit. Review by Jeff Davis.
1 parent 8c8745b commit 4da99ea

File tree

6 files changed

+74
-187
lines changed

6 files changed

+74
-187
lines changed

src/backend/access/heap/heapam.c

Lines changed: 46 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1514,6 +1514,10 @@ heap_fetch(Relation relation,
15141514
* found, we update *tid to reference that tuple's offset number, and
15151515
* return TRUE. If no match, return FALSE without modifying *tid.
15161516
*
1517+
* heapTuple is a caller-supplied buffer. When a match is found, we return
1518+
* the tuple here, in addition to updating *tid. If no match is found, the
1519+
* contents of this buffer on return are undefined.
1520+
*
15171521
* If all_dead is not NULL, we check non-visible tuples to see if they are
15181522
* globally dead; *all_dead is set TRUE if all members of the HOT chain
15191523
* are vacuumable, FALSE if not.
@@ -1524,28 +1528,31 @@ heap_fetch(Relation relation,
15241528
*/
15251529
bool
15261530
heap_hot_search_buffer(ItemPointer tid, Relation relation, Buffer buffer,
1527-
Snapshot snapshot, bool *all_dead)
1531+
Snapshot snapshot, HeapTuple heapTuple,
1532+
bool *all_dead, bool first_call)
15281533
{
15291534
Page dp = (Page) BufferGetPage(buffer);
15301535
TransactionId prev_xmax = InvalidTransactionId;
15311536
OffsetNumber offnum;
15321537
bool at_chain_start;
15331538
bool valid;
1539+
bool skip;
15341540

1541+
/* If this is not the first call, previous call returned a (live!) tuple */
15351542
if (all_dead)
1536-
*all_dead = true;
1543+
*all_dead = first_call;
15371544

15381545
Assert(TransactionIdIsValid(RecentGlobalXmin));
15391546

15401547
Assert(ItemPointerGetBlockNumber(tid) == BufferGetBlockNumber(buffer));
15411548
offnum = ItemPointerGetOffsetNumber(tid);
1542-
at_chain_start = true;
1549+
at_chain_start = first_call;
1550+
skip = !first_call;
15431551

15441552
/* Scan through possible multiple members of HOT-chain */
15451553
for (;;)
15461554
{
15471555
ItemId lp;
1548-
HeapTupleData heapTuple;
15491556

15501557
/* check for bogus TID */
15511558
if (offnum < FirstOffsetNumber || offnum > PageGetMaxOffsetNumber(dp))
@@ -1568,15 +1575,15 @@ heap_hot_search_buffer(ItemPointer tid, Relation relation, Buffer buffer,
15681575
break;
15691576
}
15701577

1571-
heapTuple.t_data = (HeapTupleHeader) PageGetItem(dp, lp);
1572-
heapTuple.t_len = ItemIdGetLength(lp);
1573-
heapTuple.t_tableOid = relation->rd_id;
1574-
heapTuple.t_self = *tid;
1578+
heapTuple->t_data = (HeapTupleHeader) PageGetItem(dp, lp);
1579+
heapTuple->t_len = ItemIdGetLength(lp);
1580+
heapTuple->t_tableOid = relation->rd_id;
1581+
heapTuple->t_self = *tid;
15751582

15761583
/*
15771584
* Shouldn't see a HEAP_ONLY tuple at chain start.
15781585
*/
1579-
if (at_chain_start && HeapTupleIsHeapOnly(&heapTuple))
1586+
if (at_chain_start && HeapTupleIsHeapOnly(heapTuple))
15801587
break;
15811588

15821589
/*
@@ -1585,43 +1592,54 @@ heap_hot_search_buffer(ItemPointer tid, Relation relation, Buffer buffer,
15851592
*/
15861593
if (TransactionIdIsValid(prev_xmax) &&
15871594
!TransactionIdEquals(prev_xmax,
1588-
HeapTupleHeaderGetXmin(heapTuple.t_data)))
1595+
HeapTupleHeaderGetXmin(heapTuple->t_data)))
15891596
break;
15901597

1591-
/* If it's visible per the snapshot, we must return it */
1592-
valid = HeapTupleSatisfiesVisibility(&heapTuple, snapshot, buffer);
1593-
CheckForSerializableConflictOut(valid, relation, &heapTuple, buffer,
1594-
snapshot);
1595-
if (valid)
1598+
/*
1599+
* When first_call is true (and thus, skip is initally false) we'll
1600+
* return the first tuple we find. But on later passes, heapTuple
1601+
* will initially be pointing to the tuple we returned last time.
1602+
* Returning it again would be incorrect (and would loop forever),
1603+
* so we skip it and return the next match we find.
1604+
*/
1605+
if (!skip)
15961606
{
1597-
ItemPointerSetOffsetNumber(tid, offnum);
1598-
PredicateLockTuple(relation, &heapTuple, snapshot);
1599-
if (all_dead)
1600-
*all_dead = false;
1601-
return true;
1607+
/* If it's visible per the snapshot, we must return it */
1608+
valid = HeapTupleSatisfiesVisibility(heapTuple, snapshot, buffer);
1609+
CheckForSerializableConflictOut(valid, relation, heapTuple,
1610+
buffer, snapshot);
1611+
if (valid)
1612+
{
1613+
ItemPointerSetOffsetNumber(tid, offnum);
1614+
PredicateLockTuple(relation, heapTuple, snapshot);
1615+
if (all_dead)
1616+
*all_dead = false;
1617+
return true;
1618+
}
16021619
}
1620+
skip = false;
16031621

16041622
/*
16051623
* If we can't see it, maybe no one else can either. At caller
16061624
* request, check whether all chain members are dead to all
16071625
* transactions.
16081626
*/
16091627
if (all_dead && *all_dead &&
1610-
HeapTupleSatisfiesVacuum(heapTuple.t_data, RecentGlobalXmin,
1628+
HeapTupleSatisfiesVacuum(heapTuple->t_data, RecentGlobalXmin,
16111629
buffer) != HEAPTUPLE_DEAD)
16121630
*all_dead = false;
16131631

16141632
/*
16151633
* Check to see if HOT chain continues past this tuple; if so fetch
16161634
* the next offnum and loop around.
16171635
*/
1618-
if (HeapTupleIsHotUpdated(&heapTuple))
1636+
if (HeapTupleIsHotUpdated(heapTuple))
16191637
{
1620-
Assert(ItemPointerGetBlockNumber(&heapTuple.t_data->t_ctid) ==
1638+
Assert(ItemPointerGetBlockNumber(&heapTuple->t_data->t_ctid) ==
16211639
ItemPointerGetBlockNumber(tid));
1622-
offnum = ItemPointerGetOffsetNumber(&heapTuple.t_data->t_ctid);
1640+
offnum = ItemPointerGetOffsetNumber(&heapTuple->t_data->t_ctid);
16231641
at_chain_start = false;
1624-
prev_xmax = HeapTupleHeaderGetXmax(heapTuple.t_data);
1642+
prev_xmax = HeapTupleHeaderGetXmax(heapTuple->t_data);
16251643
}
16261644
else
16271645
break; /* end of chain */
@@ -1643,10 +1661,12 @@ heap_hot_search(ItemPointer tid, Relation relation, Snapshot snapshot,
16431661
{
16441662
bool result;
16451663
Buffer buffer;
1664+
HeapTupleData heapTuple;
16461665

16471666
buffer = ReadBuffer(relation, ItemPointerGetBlockNumber(tid));
16481667
LockBuffer(buffer, BUFFER_LOCK_SHARE);
1649-
result = heap_hot_search_buffer(tid, relation, buffer, snapshot, all_dead);
1668+
result = heap_hot_search_buffer(tid, relation, buffer, snapshot,
1669+
&heapTuple, all_dead, true);
16501670
LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
16511671
ReleaseBuffer(buffer);
16521672
return result;

src/backend/access/index/genam.c

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -113,9 +113,7 @@ RelationGetIndexScan(Relation indexRelation, int nkeys, int norderbys)
113113
ItemPointerSetInvalid(&scan->xs_ctup.t_self);
114114
scan->xs_ctup.t_data = NULL;
115115
scan->xs_cbuf = InvalidBuffer;
116-
scan->xs_hot_dead = false;
117-
scan->xs_next_hot = InvalidOffsetNumber;
118-
scan->xs_prev_xmax = InvalidTransactionId;
116+
scan->xs_continue_hot = false;
119117

120118
return scan;
121119
}

src/backend/access/index/indexam.c

Lines changed: 21 additions & 153 deletions
Original file line numberDiff line numberDiff line change
@@ -335,7 +335,7 @@ index_rescan(IndexScanDesc scan,
335335
scan->xs_cbuf = InvalidBuffer;
336336
}
337337

338-
scan->xs_next_hot = InvalidOffsetNumber;
338+
scan->xs_continue_hot = false;
339339

340340
scan->kill_prior_tuple = false; /* for safety */
341341

@@ -417,7 +417,7 @@ index_restrpos(IndexScanDesc scan)
417417
SCAN_CHECKS;
418418
GET_SCAN_PROCEDURE(amrestrpos);
419419

420-
scan->xs_next_hot = InvalidOffsetNumber;
420+
scan->xs_continue_hot = false;
421421

422422
scan->kill_prior_tuple = false; /* for safety */
423423

@@ -443,26 +443,18 @@ index_getnext(IndexScanDesc scan, ScanDirection direction)
443443
HeapTuple heapTuple = &scan->xs_ctup;
444444
ItemPointer tid = &heapTuple->t_self;
445445
FmgrInfo *procedure;
446+
bool all_dead = false;
446447

447448
SCAN_CHECKS;
448449
GET_SCAN_PROCEDURE(amgettuple);
449450

450451
Assert(TransactionIdIsValid(RecentGlobalXmin));
451452

452-
/*
453-
* We always reset xs_hot_dead; if we are here then either we are just
454-
* starting the scan, or we previously returned a visible tuple, and in
455-
* either case it's inappropriate to kill the prior index entry.
456-
*/
457-
scan->xs_hot_dead = false;
458-
459453
for (;;)
460454
{
461-
OffsetNumber offnum;
462-
bool at_chain_start;
463-
Page dp;
455+
bool got_heap_tuple;
464456

465-
if (scan->xs_next_hot != InvalidOffsetNumber)
457+
if (scan->xs_continue_hot)
466458
{
467459
/*
468460
* We are resuming scan of a HOT chain after having returned an
@@ -471,10 +463,6 @@ index_getnext(IndexScanDesc scan, ScanDirection direction)
471463
Assert(BufferIsValid(scan->xs_cbuf));
472464
Assert(ItemPointerGetBlockNumber(tid) ==
473465
BufferGetBlockNumber(scan->xs_cbuf));
474-
Assert(TransactionIdIsValid(scan->xs_prev_xmax));
475-
offnum = scan->xs_next_hot;
476-
at_chain_start = false;
477-
scan->xs_next_hot = InvalidOffsetNumber;
478466
}
479467
else
480468
{
@@ -488,7 +476,7 @@ index_getnext(IndexScanDesc scan, ScanDirection direction)
488476
* comments in RelationGetIndexScan().
489477
*/
490478
if (!scan->xactStartedInRecovery)
491-
scan->kill_prior_tuple = scan->xs_hot_dead;
479+
scan->kill_prior_tuple = all_dead;
492480

493481
/*
494482
* The AM's gettuple proc finds the next index entry matching the
@@ -521,151 +509,31 @@ index_getnext(IndexScanDesc scan, ScanDirection direction)
521509
if (prev_buf != scan->xs_cbuf)
522510
heap_page_prune_opt(scan->heapRelation, scan->xs_cbuf,
523511
RecentGlobalXmin);
524-
525-
/* Prepare to scan HOT chain starting at index-referenced offnum */
526-
offnum = ItemPointerGetOffsetNumber(tid);
527-
at_chain_start = true;
528-
529-
/* We don't know what the first tuple's xmin should be */
530-
scan->xs_prev_xmax = InvalidTransactionId;
531-
532-
/* Initialize flag to detect if all entries are dead */
533-
scan->xs_hot_dead = true;
534512
}
535513

536514
/* Obtain share-lock on the buffer so we can examine visibility */
537515
LockBuffer(scan->xs_cbuf, BUFFER_LOCK_SHARE);
516+
got_heap_tuple = heap_hot_search_buffer(tid, scan->heapRelation,
517+
scan->xs_cbuf,
518+
scan->xs_snapshot,
519+
&scan->xs_ctup,
520+
&all_dead,
521+
!scan->xs_continue_hot);
522+
LockBuffer(scan->xs_cbuf, BUFFER_LOCK_UNLOCK);
538523

539-
dp = (Page) BufferGetPage(scan->xs_cbuf);
540-
541-
/* Scan through possible multiple members of HOT-chain */
542-
for (;;)
524+
if (got_heap_tuple)
543525
{
544-
ItemId lp;
545-
ItemPointer ctid;
546-
bool valid;
547-
548-
/* check for bogus TID */
549-
if (offnum < FirstOffsetNumber ||
550-
offnum > PageGetMaxOffsetNumber(dp))
551-
break;
552-
553-
lp = PageGetItemId(dp, offnum);
554-
555-
/* check for unused, dead, or redirected items */
556-
if (!ItemIdIsNormal(lp))
557-
{
558-
/* We should only see a redirect at start of chain */
559-
if (ItemIdIsRedirected(lp) && at_chain_start)
560-
{
561-
/* Follow the redirect */
562-
offnum = ItemIdGetRedirect(lp);
563-
at_chain_start = false;
564-
continue;
565-
}
566-
/* else must be end of chain */
567-
break;
568-
}
569-
570-
/*
571-
* We must initialize all of *heapTuple (ie, scan->xs_ctup) since
572-
* it is returned to the executor on success.
573-
*/
574-
heapTuple->t_data = (HeapTupleHeader) PageGetItem(dp, lp);
575-
heapTuple->t_len = ItemIdGetLength(lp);
576-
ItemPointerSetOffsetNumber(tid, offnum);
577-
heapTuple->t_tableOid = RelationGetRelid(scan->heapRelation);
578-
ctid = &heapTuple->t_data->t_ctid;
579-
580526
/*
581-
* Shouldn't see a HEAP_ONLY tuple at chain start. (This test
582-
* should be unnecessary, since the chain root can't be removed
583-
* while we have pin on the index entry, but let's make it
584-
* anyway.)
527+
* Only in a non-MVCC snapshot can more than one member of the
528+
* HOT chain be visible.
585529
*/
586-
if (at_chain_start && HeapTupleIsHeapOnly(heapTuple))
587-
break;
588-
589-
/*
590-
* The xmin should match the previous xmax value, else chain is
591-
* broken. (Note: this test is not optional because it protects
592-
* us against the case where the prior chain member's xmax aborted
593-
* since we looked at it.)
594-
*/
595-
if (TransactionIdIsValid(scan->xs_prev_xmax) &&
596-
!TransactionIdEquals(scan->xs_prev_xmax,
597-
HeapTupleHeaderGetXmin(heapTuple->t_data)))
598-
break;
599-
600-
/* If it's visible per the snapshot, we must return it */
601-
valid = HeapTupleSatisfiesVisibility(heapTuple, scan->xs_snapshot,
602-
scan->xs_cbuf);
603-
604-
CheckForSerializableConflictOut(valid, scan->heapRelation,
605-
heapTuple, scan->xs_cbuf,
606-
scan->xs_snapshot);
607-
608-
if (valid)
609-
{
610-
/*
611-
* If the snapshot is MVCC, we know that it could accept at
612-
* most one member of the HOT chain, so we can skip examining
613-
* any more members. Otherwise, check for continuation of the
614-
* HOT-chain, and set state for next time.
615-
*/
616-
if (IsMVCCSnapshot(scan->xs_snapshot))
617-
scan->xs_next_hot = InvalidOffsetNumber;
618-
else if (HeapTupleIsHotUpdated(heapTuple))
619-
{
620-
Assert(ItemPointerGetBlockNumber(ctid) ==
621-
ItemPointerGetBlockNumber(tid));
622-
scan->xs_next_hot = ItemPointerGetOffsetNumber(ctid);
623-
scan->xs_prev_xmax = HeapTupleHeaderGetXmax(heapTuple->t_data);
624-
}
625-
else
626-
scan->xs_next_hot = InvalidOffsetNumber;
627-
628-
PredicateLockTuple(scan->heapRelation, heapTuple, scan->xs_snapshot);
629-
630-
LockBuffer(scan->xs_cbuf, BUFFER_LOCK_UNLOCK);
631-
632-
pgstat_count_heap_fetch(scan->indexRelation);
633-
634-
return heapTuple;
635-
}
636-
637-
/*
638-
* If we can't see it, maybe no one else can either. Check to see
639-
* if the tuple is dead to all transactions. If we find that all
640-
* the tuples in the HOT chain are dead, we'll signal the index AM
641-
* to not return that TID on future indexscans.
642-
*/
643-
if (scan->xs_hot_dead &&
644-
HeapTupleSatisfiesVacuum(heapTuple->t_data, RecentGlobalXmin,
645-
scan->xs_cbuf) != HEAPTUPLE_DEAD)
646-
scan->xs_hot_dead = false;
647-
648-
/*
649-
* Check to see if HOT chain continues past this tuple; if so
650-
* fetch the next offnum (we don't bother storing it into
651-
* xs_next_hot, but must store xs_prev_xmax), and loop around.
652-
*/
653-
if (HeapTupleIsHotUpdated(heapTuple))
654-
{
655-
Assert(ItemPointerGetBlockNumber(ctid) ==
656-
ItemPointerGetBlockNumber(tid));
657-
offnum = ItemPointerGetOffsetNumber(ctid);
658-
at_chain_start = false;
659-
scan->xs_prev_xmax = HeapTupleHeaderGetXmax(heapTuple->t_data);
660-
}
661-
else
662-
break; /* end of chain */
663-
} /* loop over a single HOT chain */
664-
665-
LockBuffer(scan->xs_cbuf, BUFFER_LOCK_UNLOCK);
530+
scan->xs_continue_hot = !IsMVCCSnapshot(scan->xs_snapshot);
531+
pgstat_count_heap_fetch(scan->indexRelation);
532+
return heapTuple;
533+
}
666534

667535
/* Loop around to ask index AM for another TID */
668-
scan->xs_next_hot = InvalidOffsetNumber;
536+
scan->xs_continue_hot = false;
669537
}
670538

671539
/* Release any held pin on a heap page */

0 commit comments

Comments
 (0)