From: | Andres Freund <andres(at)anarazel(dot)de> |
---|---|
To: | Petr Jelinek <petr(dot)jelinek(at)2ndquadrant(dot)com> |
Cc: | Robert Haas <robertmhaas(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, Jeff Janes <jeff(dot)janes(at)gmail(dot)com> |
Subject: | Re: logical replication busy-waiting on a lock |
Date: | 2017-06-03 01:25:48 |
Message-ID: | 20170603012548.q564epwswmdy77zb@alap3.anarazel.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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(at)pjmodos(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Thomas Munro | 2017-06-03 01:46:00 | Re: PG10 transition tables, wCTEs and multiple operations on the same table |
Previous Message | Petr Jelinek | 2017-06-03 01:03:20 | Re: logical replication busy-waiting on a lock |