Re: pgsql: Avoid duplicate XIDs at recovery when building initial snapshot

Hi,

On 2018-10-22 12:36:25 -0300, Alvaro Herrera wrote:
> On 2018-Oct-14, Andres Freund wrote:
>
> > On 2018-10-14 13:26:24 +0000, Michael Paquier wrote:
> > > Avoid duplicate XIDs at recovery when building initial snapshot
>
> > I'm unhappy this approach was taken over objections. Without a real
> > warning. Even leaving the crummyness aside, did you check other users
> > of XLOG_RUNNING_XACTS, e.g. logical decoding?
>
> Mumble. Is there a real problem here -- I mean, did this break logical
> decoding? Maybe we need some more tests to ensure this stuff works
> sanely for logical decoding.

Hm? My point is that this fix just puts a band-aid onto *one* of the
places that read a XLOG_RUNNING_XACTS. Which still leaves the contents
of WAL record corrupted. There's not even a note at the WAL-record's
definition or its logging denoting that the contents are not what you'd
expect. I don't mean that the fix would break logical decoding, but
that it's possible that an equivalent of the problem affecting hot
standby also affects logical decoding. And even leaving those two users
aside, it's possible that there will be further vulernable internal
users or extensions parsing the WAL.

> FWIW and not directly related, I recently became aware (because of a
> customer question) that txid_current_snapshot() is oblivious of
> XLOG_RUNNING_XACTS in a standby. AFAICS it's not a really serious
> concern, but it was surprising.

That's more fundamental than just XLOG_RUNNING_XACTS though, no? The
whole way running transactions (i.e. including those that are just
detected by looking at their xid) are handled in the known xids struct
and in snapshots seems incompatible with that, no?

Greetings,

Andres Freund