From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andres Freund <andres@2ndquadrant.com>
cc: Josh Berkus <josh@agliodbs.com>, Jeff Janes <jeff.janes@gmail.com>,
        "pgsql-performance@postgresql.org" <pgsql-performance@postgresql.org>,
        Bart³omiej Romañski <br@sentia.pl>
Subject: Re: Planner performance extremely affected by an hanging transaction
 (20-30 times)?
In-reply-to: <20130927211439.GB7260@awork2.anarazel.de>
References: <5815.1380018947@sss.pgh.pa.us>
 <e8795f5e292a0bd1a4d8ebbcb54e4a98.squirrel@shrek.krogh.cc>
 <WM!41fcd3fa079d542735fe69d3254859689c1085f7c897fa6e9a9b828ad9d2c80ba7b6b2c4c67c942a6115262993147370!@asav-1.01.com>
 <5241CF2A.2070309@agliodbs.com>
 <CAMkU=1ys-fB2Nczt=LT1pVHzK3Jv4Jb-67N29Gzyx3nraweoZQ@mail.gmail.com>
 <20130925175312.GB5578@awork2.anarazel.de>
 <CAMkU=1yy-YEQVvqj2xJitT1EFkyuFk7uTV_hrOMGyGMxpU=N+Q@mail.gmail.com>
 <20130925184330.GC5578@awork2.anarazel.de>
 <WM!93b9a5691f0628ebed5547d1ebe916326a0f388cbdc28742bef1b0c7862375a07b801f17c707cbdf6a3104b7920696af!@asav-1.01.com>
 <5245F11E.20804@agliodbs.com> <20130927211439.GB7260@awork2.anarazel.de>
Comments: In-reply-to Andres Freund <andres@2ndquadrant.com>
 message dated "Fri, 27 Sep 2013 23:14:39 +0200"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <11889.1384199192.0@sss.pgh.pa.us>
Date: Mon, 11 Nov 2013 14:48:14 -0500
Message-ID: <11927.1384199294@sss.pgh.pa.us>
Precedence: bulk
Sender: pgsql-performance-owner@postgresql.org

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <11889.1384199192.1@sss.pgh.pa.us>

Andres Freund <andres@2ndquadrant.com> writes:
> Also, this really isn't going to fix the issue discussed here - this was
> just about the additional ProcArrayLock contention. I don't think it
> would change anything dramatical in your case.

All of these proposals are pretty scary for back-patching purposes,
anyway.  I think what we should consider doing is just changing
get_actual_variable_range() to use a cheaper snapshot type, as in
the attached patch (which is for 9.3 but applies to 9.2 with slight
offset).  On my machine, this seems to make the pathological behavior
in BR's test case go away just fine.  I'd be interested to hear what
it does in the real-world scenarios being complained of.

			regards, tom lane


------- =_aaaaaaaaaa0
Content-Type: text/x-diff; name="get-actual-variable-range-cheaper.patch";
 charset="us-ascii"
Content-ID: <11889.1384199192.2@sss.pgh.pa.us>
Content-Description: get-actual-variable-range-cheaper.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfu=
ncs.c
index d8c1a88..d1b9473 100644
*** a/src/backend/utils/adt/selfuncs.c
--- b/src/backend/utils/adt/selfuncs.c
*************** get_actual_variable_range(PlannerInfo *r
*** 4951,4956 ****
--- 4951,4957 ----
  			HeapTuple	tup;
  			Datum		values[INDEX_MAX_KEYS];
  			bool		isnull[INDEX_MAX_KEYS];
+ 			SnapshotData SnapshotDirty;
=20=20
  			estate =3D CreateExecutorState();
  			econtext =3D GetPerTupleExprContext(estate);
*************** get_actual_variable_range(PlannerInfo *r
*** 4973,4978 ****
--- 4974,4980 ----
  			slot =3D MakeSingleTupleTableSlot(RelationGetDescr(heapRel));
  			econtext->ecxt_scantuple =3D slot;
  			get_typlenbyval(vardata->atttype, &typLen, &typByVal);
+ 			InitDirtySnapshot(SnapshotDirty);
=20=20
  			/* set up an IS NOT NULL scan key so that we ignore nulls */
  			ScanKeyEntryInitialize(&scankeys[0],
*************** get_actual_variable_range(PlannerInfo *r
*** 4989,4996 ****
  			/* If min is requested ... */
  			if (min)
  			{
! 				index_scan =3D index_beginscan(heapRel, indexRel, SnapshotNow,
! 											 1, 0);
  				index_rescan(index_scan, scankeys, 1, NULL, 0);
=20=20
  				/* Fetch first tuple in sortop's direction */
--- 4991,5013 ----
  			/* If min is requested ... */
  			if (min)
  			{
! 				/*
! 				 * 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 =3D index_beginscan(heapRel, indexRel,
! 											 &SnapshotDirty, 1, 0);
  				index_rescan(index_scan, scankeys, 1, NULL, 0);
=20=20
  				/* Fetch first tuple in sortop's direction */
*************** get_actual_variable_range(PlannerInfo *r
*** 5021,5028 ****
  			/* If max is requested, and we didn't find the index is empty */
  			if (max && have_data)
  			{
! 				index_scan =3D index_beginscan(heapRel, indexRel, SnapshotNow,
! 											 1, 0);
  				index_rescan(index_scan, scankeys, 1, NULL, 0);
=20=20
  				/* Fetch first tuple in reverse direction */
--- 5038,5045 ----
  			/* If max is requested, and we didn't find the index is empty */
  			if (max && have_data)
  			{
! 				index_scan =3D index_beginscan(heapRel, indexRel,
! 											 &SnapshotDirty, 1, 0);
  				index_rescan(index_scan, scankeys, 1, NULL, 0);
=20=20
  				/* Fetch first tuple in reverse direction */

------- =_aaaaaaaaaa0
Content-Type: text/plain
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0


-- 
Sent via pgsql-performance mailing list (pgsql-performance@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-performance

------- =_aaaaaaaaaa0--