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
signature.asc
Description: PGP signature
