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 <pjmodos@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
