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
40608e7f949fb7e4025c0ddd5be01939adc79eec).

diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index 7cf4995dec7820f1c706873b81eeef6f9b47ce89..9982bf54d4bf63461564a96af75ac15c82f1433e 100644 (file)
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -4961,6 +4961,7 @@ get_actual_variable_range(PlannerInfo *root, VariableStatData *vardata,
                        HeapTuple       tup;
                        Datum           values[INDEX_MAX_KEYS];
                        bool            isnull[INDEX_MAX_KEYS];
+                       SnapshotData SnapshotDirty;
 
                        estate = CreateExecutorState();
                        econtext = GetPerTupleExprContext(estate);
@@ -4983,6 +4984,7 @@ get_actual_variable_range(PlannerInfo *root, VariableStatData *vardata,
                        slot = MakeSingleTupleTableSlot(RelationGetDescr(heapRel));
                        econtext->ecxt_scantuple = slot;
                        get_typlenbyval(vardata->atttype, &typLen, &typByVal);
+                       InitDirtySnapshot(SnapshotDirty);
 
                        /* set up an IS NOT NULL scan key so that we ignore nulls */
                        ScanKeyEntryInitialize(&scankeys[0],
@@ -4999,8 +5001,23 @@ get_actual_variable_range(PlannerInfo *root, VariableStatData *vardata,
                        /* If min is requested ... */
                        if (min)
                        {
-                               index_scan = index_beginscan(heapRel, indexRel, SnapshotNow,
-                                                                                        1, 0);
+                               /*
+                                * In principle, we should scan the index with our current
+                                * active snapshot, which is the best approximation we've got
+                                * to what the query will see when executed.  But that won't
+                                * be exact if a new snap is taken before running the query,
+                                * and it can be very expensive if a lot of uncommitted rows
+                                * exist at the end of the index (because we'll laboriously
+                                * fetch each one and reject it).  What seems like a good
+                                * compromise is to use SnapshotDirty.  That will accept
+                                * uncommitted rows, and thus avoid fetching multiple heap
+                                * tuples in this scenario.  On the other hand, it will reject
+                                * known-dead rows, and thus not give a bogus answer when the
+                                * extreme value has been deleted; that case motivates not
+                                * using SnapshotAny here.
+                                */
+                               index_scan = index_beginscan(heapRel, indexRel,
+                                                                                        &SnapshotDirty, 1, 0);
                                index_rescan(index_scan, scankeys, 1, NULL, 0);
 
                                /* Fetch first tuple in sortop's direction */
@@ -5031,8 +5048,8 @@ get_actual_variable_range(PlannerInfo *root, VariableStatData *vardata,
                        /* If max is requested, and we didn't find the index is empty */
                        if (max && have_data)
                        {
-                               index_scan = index_beginscan(heapRel, indexRel, SnapshotNow,
-                                                                                        1, 0);
+                               index_scan = index_beginscan(heapRel, indexRel,
+                                                                                        &SnapshotDirty, 1, 0);
                                index_rescan(index_scan, scankeys, 1, NULL, 0);
 
                                /* Fetch first tuple in reverse direction */