From: | Michael Paquier <michael(at)paquier(dot)xyz> |
---|---|
To: | Simon Riggs <simon(at)2ndquadrant(dot)com> |
Cc: | Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: pgsql: Avoid duplicate XIDs at recovery when building initial snapshot |
Date: | 2018-11-30 23:08:04 |
Message-ID: | 20181130230804.GC1846@paquier.xyz |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-committers pgsql-hackers |
On Fri, Nov 30, 2018 at 02:55:47PM +0000, Simon Riggs wrote:
> 1df21ddb looks OK to me and was simple enough to backpatch safely.
Thanks for the feedback!
> Seems excessive to say that the WAL record is corrupt, it just contains
> duplicates, just as exported snapshots do. There's a few other imprecise
> things around in WAL, that is why we need the RunningXact data in the first
> place. So we have a choice of whether to remove the duplicates eagerly or
> lazily.
>
> For GetRunningTransactionData(), we can do eager or lazy, since its not a
> foreground process. I don't object to changing it to be eager in this path,
> but this patch is more complex than 1df21ddb and I don't think we should
> backpatch this change, assuming it is acceptable.
Yes, I would avoid a backpatch for this more complicated one, and
we need a solution for already-generated WAL. It is not complicated to
handle duplicates for xacts and subxacts however holding ProcArrayLock
for a longer time stresses me as it is already a bottleneck.
> This patch doesn't do it, but the suggestion that we touch
> GetSnapshotData() in the same way so we de-duplicate eagerly is a different
> matter and would need careful performance testing to ensure we don't slow
> down 2PC users.
Definitely. That's a much hotter code path. I am not completely sure
if that's an effort worth pursuing either..
--
Michael
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2018-12-01 07:16:44 | Re: pgsql: Switch pg_verify_checksums back to a blacklist |
Previous Message | Michael Paquier | 2018-11-30 23:00:16 | Re: pgsql: Switch pg_verify_checksums back to a blacklist |
From | Date | Subject | |
---|---|---|---|
Next Message | Stephen Frost | 2018-11-30 23:11:03 | Re: [PATCH] Log CSV by default |
Previous Message | Michael Paquier | 2018-11-30 23:03:13 | Re: Add extension options to control TAP and isolation tests |