From: Heikki Linnakangas <hlinnakangas@vmware.com>
To: Mark Kirkwood <mark.kirkwood@catalyst.net.nz>
Cc: Simon Riggs <simon@2ndQuadrant.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Gavin Flower <gavinflower@archidevsys.co.nz>
Cc: pgsql-performance@postgresql.org <pgsql-performance@postgresql.org>
Subject: Re: In progress INSERT wrecks plans on table
Date: Sun, 16 Jun 2013 18:23:20 +0300
Message-ID: <51BDD868.3020106@vmware.com> (raw)
In-Reply-To: <51870CB6.6090404@catalyst.net.nz>
References: <5179E77B.7050601@catalyst.net.nz>
<5179ECC5.2010206@archidevsys.co.nz>
<5179F259.3010509@catalyst.net.nz>
<5179F5C6.8070702@archidevsys.co.nz>
<5180E3C8.3010201@catalyst.net.nz>
<2121.1367417215@sss.pgh.pa.us>
<5181B80D.70404@catalyst.net.nz>
<CA+U5nMKYYZDekhtxz1E1B11dAm9UJoHBQHrB6DiiZDJo3fLd0Q@mail.gmail.com>
<916bff05a5ab130c76d7835abbaf7762.squirrel@mail.catalyst.net.nz>
<CA+U5nMJHjQccVSB3SkAHdRn2AfuuO49wA8-ZGB366cddTynjdQ@mail.gmail.com>
<CA+U5nMKMfZUEimZ9=oF2wJ-WkiGhMw3mQjUFT6PBGofUpmXL8A@mail.gmail.com>
<51870CB6.6090404@catalyst.net.nz>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-performance>
On 06.05.2013 04:51, Mark Kirkwood wrote:
> On 05/05/13 00:49, Simon Riggs wrote:
>> On 3 May 2013 13:41, Simon Riggs <simon@2ndquadrant.com> wrote:
>>
>>> (3) to make the check on TransactionIdIsInProgress() into a heuristic,
>>> since we don't *need* to check that, so if we keep checking the same
>>> xid repeatedly we can reduce the number of checks or avoid xids that
>>> seem to be long running. That's slightly more coding than my quick
>>> hack here but seems worth it.
>>>
>>> I think we need both (1) and (3) but the attached patch does just (1).
>>>
>>> This is a similar optimisation to the one I introduced for
>>> TransactionIdIsKnownCompleted(), except this applies to repeated
>>> checking of as yet-incomplete xids, and to bulk concurrent
>>> transactions.
>>
>> ISTM we can improve performance of TransactionIdIsInProgress() by
>> caching the procno of our last xid.
>>
>> Mark, could you retest with both these patches? Thanks.
>>
>
> Thanks Simon, will do and report back.
Did anyone ever try (3) ?
I'm not sure if this the same idea as (3) above, but ISTM that
HeapTupleSatisfiesMVCC doesn't actually need to call
TransactionIdIsInProgress(), because it checks XidInMVCCSnapshot(). The
comment at the top of tqual.c says:
> * NOTE: must check TransactionIdIsInProgress (which looks in PGXACT array)
> * before TransactionIdDidCommit/TransactionIdDidAbort (which look in
> * pg_clog). Otherwise we have a race condition: we might decide that a
> * just-committed transaction crashed, because none of the tests succeed.
> * xact.c is careful to record commit/abort in pg_clog before it unsets
> * MyPgXact->xid in PGXACT array. That fixes that problem, but it also
> * means there is a window where TransactionIdIsInProgress and
> * TransactionIdDidCommit will both return true. If we check only
> * TransactionIdDidCommit, we could consider a tuple committed when a
> * later GetSnapshotData call will still think the originating transaction
> * is in progress, which leads to application-level inconsistency. The
> * upshot is that we gotta check TransactionIdIsInProgress first in all
> * code paths, except for a few cases where we are looking at
> * subtransactions of our own main transaction and so there can't be any
> * race condition.
If TransactionIdIsInProgress() returns true for a given XID, then surely
it was also running when the snapshot was taken (or had not even began
yet). In which case the XidInMVCCSnapshot() call will also return true.
Am I missing something?
There's one little problem: we currently only set the hint bits when
TransactionIdIsInProgress() returns false. If we do that earlier, then
even though HeapTupleSatisfiesMVCC works correctly thanks to the
XidInMVCCSnapshot call, other HeapTupleSatisfies* functions that don't
call XIdInMVCCSnapshot might see the tuple as committed or aborted too
early, if they see the hint bit as set while the transaction is still
in-progress according to the proc array. Would have to check all the
callers of those other HeapTupleSatisfies* functions to verify if that's OK.
- Heikki
--
Sent via pgsql-performance mailing list (pgsql-performance@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-performance
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-performance@postgresql.org
Cc: hlinnakangas@vmware.com, mark.kirkwood@catalyst.net.nz, simon@2ndQuadrant.com, tgl@sss.pgh.pa.us, gavinflower@archidevsys.co.nz
Subject: Re: In progress INSERT wrecks plans on table
In-Reply-To: <51BDD868.3020106@vmware.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox