Hi,

On 2017-06-03 03:03:20 +0200, Petr Jelinek wrote:
> All right, here is my rough attempt on doing what Andres has suggested,
> going straight from xid to vxid, seems to work fine in my tests and
> parallel pg_dump seems happy with it as well.

Cool!


> The second patch removes the xid parameter from SnapBuildBuildSnapshot
> as it's not used for anything and skips the GetTopTransactionId() call
> as well.

Makes sense.


> That should solve the original problem reported here.

Did you verify that?


> From 4f7eb5b8fdbab2539731414e81d7a7419a83e5b1 Mon Sep 17 00:00:00 2001
> From: Petr Jelinek <pjmo...@pjmodos.net>
> Date: Sat, 3 Jun 2017 02:06:22 +0200
> Subject: [PATCH 1/2] Use virtual transaction instead of normal ones in
>  exported snapshots

> @@ -1817,8 +1818,10 @@ ProcArrayInstallImportedXmin(TransactionId xmin, 
> TransactionId sourcexid)
>               if (pgxact->vacuumFlags & PROC_IN_VACUUM)
>                       continue;
>  
> -             xid = pgxact->xid;              /* fetch just once */

Hm, I don't quite understand what the 'fetch just once' bit was trying
to achieve here (not to speak of the fact that it's simply not
guaranteed this way).  Therefore I think you're right to simply
disregard it.


> @@ -1741,17 +1741,17 @@ GetSerializableTransactionSnapshotInt(Snapshot 
> snapshot,
>       } while (!sxact);
>  
>       /* Get the snapshot, or check that it's safe to use */
> -     if (!TransactionIdIsValid(sourcexid))
> +     if (!sourcevxid)
>               snapshot = GetSnapshotData(snapshot);
> -     else if (!ProcArrayInstallImportedXmin(snapshot->xmin, sourcexid))
> +     else if (!ProcArrayInstallImportedXmin(snapshot->xmin, sourcevxid))
>       {
>               ReleasePredXact(sxact);
>               LWLockRelease(SerializableXactHashLock);
>               ereport(ERROR,
>                               
> (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
>                                errmsg("could not import the requested 
> snapshot"),
> -                        errdetail("The source transaction %u is not running 
> anymore.",
> -                                              sourcexid)));
> +        errdetail("The source virtual transaction %d/%u is not running 
> anymore.",
> +                                      sourcevxid->backendId, 
> sourcevxid->localTransactionId)));

Hm, this is a harder to read.  Wonder if we should add a pid field, that'd
make it a bit easier to interpret?


- Andres


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

Reply via email to