mirror of
https://github.com/postgres/postgres.git
synced 2025-11-07 19:06:32 +03:00
Fix BRIN to use SnapshotAny during summarization
For correctness of summarization results, it is critical that the snapshot used during the summarization scan is able to see all tuples that are live to all transactions -- including tuples inserted or deleted by in-progress transactions. Otherwise, it would be possible for a transaction to insert a tuple, then idle for a long time while a concurrent transaction executes summarization of the range: this would result in the inserted value not being considered in the summary. Previously we were trying to use a MVCC snapshot in conjunction with adding a "placeholder" tuple in the index: the snapshot would see all committed tuples, and the placeholder tuple would catch insertions by any new inserters. The hole is that prior insertions by transactions that are still in progress by the time the MVCC snapshot was taken were ignored. Kevin Grittner reported this as a bogus error message during vacuum with default transaction isolation mode set to repeatable read (because the error report mentioned a function name not being invoked during), but the problem is larger than that. To fix, tweak IndexBuildHeapRangeScan to have a new mode that behaves the way we need using SnapshotAny visibility rules. This change simplifies the BRIN code a bit, mainly by removing large comments that were mistaken. Instead, rely on the SnapshotAny semantics to provide what it needs. (The business about a placeholder tuple needs to remain: that covers the case that a transaction inserts a a tuple in a page that summarization already scanned.) Discussion: https://www.postgresql.org/message-id/20150731175700.GX2441@postgresql.org In passing, remove a couple of unused declarations from brin.h and reword a comment to be proper English. This part submitted by Kevin Grittner. Backpatch to 9.5, where BRIN was introduced.
This commit is contained in:
@@ -2161,6 +2161,7 @@ IndexBuildHeapScan(Relation heapRelation,
|
||||
{
|
||||
return IndexBuildHeapRangeScan(heapRelation, indexRelation,
|
||||
indexInfo, allow_sync,
|
||||
false,
|
||||
0, InvalidBlockNumber,
|
||||
callback, callback_state);
|
||||
}
|
||||
@@ -2170,12 +2171,17 @@ IndexBuildHeapScan(Relation heapRelation,
|
||||
* number of blocks are scanned. Scan to end-of-rel can be signalled by
|
||||
* passing InvalidBlockNumber as numblocks. Note that restricting the range
|
||||
* to scan cannot be done when requesting syncscan.
|
||||
*
|
||||
* When "anyvisible" mode is requested, all tuples visible to any transaction
|
||||
* are considered, including those inserted or deleted by transactions that are
|
||||
* still in progress.
|
||||
*/
|
||||
double
|
||||
IndexBuildHeapRangeScan(Relation heapRelation,
|
||||
Relation indexRelation,
|
||||
IndexInfo *indexInfo,
|
||||
bool allow_sync,
|
||||
bool anyvisible,
|
||||
BlockNumber start_blockno,
|
||||
BlockNumber numblocks,
|
||||
IndexBuildCallback callback,
|
||||
@@ -2209,6 +2215,12 @@ IndexBuildHeapRangeScan(Relation heapRelation,
|
||||
checking_uniqueness = (indexInfo->ii_Unique ||
|
||||
indexInfo->ii_ExclusionOps != NULL);
|
||||
|
||||
/*
|
||||
* "Any visible" mode is not compatible with uniqueness checks; make sure
|
||||
* only one of those is requested.
|
||||
*/
|
||||
Assert(!(anyvisible && checking_uniqueness));
|
||||
|
||||
/*
|
||||
* Need an EState for evaluation of index expressions and partial-index
|
||||
* predicates. Also a slot to hold the current tuple.
|
||||
@@ -2236,6 +2248,9 @@ IndexBuildHeapRangeScan(Relation heapRelation,
|
||||
{
|
||||
snapshot = RegisterSnapshot(GetTransactionSnapshot());
|
||||
OldestXmin = InvalidTransactionId; /* not used */
|
||||
|
||||
/* "any visible" mode is not compatible with this */
|
||||
Assert(!anyvisible);
|
||||
}
|
||||
else
|
||||
{
|
||||
@@ -2363,6 +2378,17 @@ IndexBuildHeapRangeScan(Relation heapRelation,
|
||||
break;
|
||||
case HEAPTUPLE_INSERT_IN_PROGRESS:
|
||||
|
||||
/*
|
||||
* In "anyvisible" mode, this tuple is visible and we don't
|
||||
* need any further checks.
|
||||
*/
|
||||
if (anyvisible)
|
||||
{
|
||||
indexIt = true;
|
||||
tupleIsAlive = true;
|
||||
break;
|
||||
}
|
||||
|
||||
/*
|
||||
* Since caller should hold ShareLock or better, normally
|
||||
* the only way to see this is if it was inserted earlier
|
||||
@@ -2409,8 +2435,16 @@ IndexBuildHeapRangeScan(Relation heapRelation,
|
||||
|
||||
/*
|
||||
* As with INSERT_IN_PROGRESS case, this is unexpected
|
||||
* unless it's our own deletion or a system catalog.
|
||||
* unless it's our own deletion or a system catalog;
|
||||
* but in anyvisible mode, this tuple is visible.
|
||||
*/
|
||||
if (anyvisible)
|
||||
{
|
||||
indexIt = true;
|
||||
tupleIsAlive = false;
|
||||
break;
|
||||
}
|
||||
|
||||
xwait = HeapTupleHeaderGetUpdateXid(heapTuple->t_data);
|
||||
if (!TransactionIdIsCurrentTransactionId(xwait))
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user