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

Commit 4162a55

Browse files
committed
Use SnapshotDirty rather than an active snapshot to probe index endpoints.
If there are lots of uncommitted tuples at the end of the index range, get_actual_variable_range() ends up fetching each one and doing an MVCC visibility check on it, until it finally hits a visible tuple. This is bad enough in isolation, considering that we don't need an exact answer only an approximate one. But because the tuples are not yet committed, each visibility check does a TransactionIdIsInProgress() test, which involves scanning the ProcArray. When multiple sessions do this concurrently, the ensuing contention results in horrid performance loss. 20X overall throughput loss on not-too-complicated queries is easy to demonstrate in the back branches (though someone's made it noticeably less bad in HEAD). We can dodge the problem fairly effectively by using SnapshotDirty rather than a normal MVCC snapshot. This will cause the index probe to take uncommitted tuples as good, so that we incur only one tuple fetch and test even if there are many such tuples. The extent to which this degrades the estimate is debatable: it's possible the result is actually a more accurate prediction than before, if the endmost tuple has become committed by the time we actually execute the query being planned. In any case, it's not very likely that it makes the estimate a lot worse. SnapshotDirty will still reject tuples that are known committed dead, so we won't give bogus answers if an invalid outlier has been deleted but not yet vacuumed from the index. (Because btrees know how to mark such tuples dead in the index, we shouldn't have a big performance problem in the case that there are many of them at the end of the range.) This consideration motivates not using SnapshotAny, which was also considered as a fix. Note: the back branches were using SnapshotNow instead of an MVCC snapshot, but the problem and solution are the same. Per performance complaints from Bartlomiej Romanski, Josh Berkus, and others. Back-patch to 9.0, where the issue was introduced (by commit 40608e7).
1 parent e8655a7 commit 4162a55

File tree

1 file changed

+21
-4
lines changed

1 file changed

+21
-4
lines changed

src/backend/utils/adt/selfuncs.c

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4961,6 +4961,7 @@ get_actual_variable_range(PlannerInfo *root, VariableStatData *vardata,
49614961
HeapTuple tup;
49624962
Datum values[INDEX_MAX_KEYS];
49634963
bool isnull[INDEX_MAX_KEYS];
4964+
SnapshotData SnapshotDirty;
49644965

49654966
estate = CreateExecutorState();
49664967
econtext = GetPerTupleExprContext(estate);
@@ -4983,6 +4984,7 @@ get_actual_variable_range(PlannerInfo *root, VariableStatData *vardata,
49834984
slot = MakeSingleTupleTableSlot(RelationGetDescr(heapRel));
49844985
econtext->ecxt_scantuple = slot;
49854986
get_typlenbyval(vardata->atttype, &typLen, &typByVal);
4987+
InitDirtySnapshot(SnapshotDirty);
49864988

49874989
/* set up an IS NOT NULL scan key so that we ignore nulls */
49884990
ScanKeyEntryInitialize(&scankeys[0],
@@ -4999,8 +5001,23 @@ get_actual_variable_range(PlannerInfo *root, VariableStatData *vardata,
49995001
/* If min is requested ... */
50005002
if (min)
50015003
{
5002-
index_scan = index_beginscan(heapRel, indexRel, SnapshotNow,
5003-
1, 0);
5004+
/*
5005+
* In principle, we should scan the index with our current
5006+
* active snapshot, which is the best approximation we've got
5007+
* to what the query will see when executed. But that won't
5008+
* be exact if a new snap is taken before running the query,
5009+
* and it can be very expensive if a lot of uncommitted rows
5010+
* exist at the end of the index (because we'll laboriously
5011+
* fetch each one and reject it). What seems like a good
5012+
* compromise is to use SnapshotDirty. That will accept
5013+
* uncommitted rows, and thus avoid fetching multiple heap
5014+
* tuples in this scenario. On the other hand, it will reject
5015+
* known-dead rows, and thus not give a bogus answer when the
5016+
* extreme value has been deleted; that case motivates not
5017+
* using SnapshotAny here.
5018+
*/
5019+
index_scan = index_beginscan(heapRel, indexRel,
5020+
&SnapshotDirty, 1, 0);
50045021
index_rescan(index_scan, scankeys, 1, NULL, 0);
50055022

50065023
/* Fetch first tuple in sortop's direction */
@@ -5031,8 +5048,8 @@ get_actual_variable_range(PlannerInfo *root, VariableStatData *vardata,
50315048
/* If max is requested, and we didn't find the index is empty */
50325049
if (max && have_data)
50335050
{
5034-
index_scan = index_beginscan(heapRel, indexRel, SnapshotNow,
5035-
1, 0);
5051+
index_scan = index_beginscan(heapRel, indexRel,
5052+
&SnapshotDirty, 1, 0);
50365053
index_rescan(index_scan, scankeys, 1, NULL, 0);
50375054

50385055
/* Fetch first tuple in reverse direction */

0 commit comments

Comments
 (0)