Re: Problem with txid_snapshot_in/out() functionality

On 04/12/14 03:27, Heikki Linnakangas wrote:
> On 04/12/2014 12:07 AM, Jan Wieck wrote:
>> Hackers,
>>
>> the Slony team has been getting seldom reports of a problem with the
>> txid_snapshot data type.
>>
>> The symptom is that a txid_snapshot on output lists the same txid
>> multiple times in the xip list part of the external representation. This
>> string is later rejected by txid_snapshot_in() when trying to determine
>> if a particular txid is visible in that snapshot using the
>> txid_visible_in_snapshot() function.
>>
>> I was not yet able to reproduce this problem in a lab environment. It
>> might be related to subtransactions and/or two phase commit (at least
>> one user is using both of them). The reported PostgreSQL version
>> involved in that case was 9.1.
>
> It's two-phase commit. When preparing a transaction, the state of the
> transaction is first transfered to a dummy PGXACT entry, and then the
> PGXACT entry of the backend is cleared. There is a transient state when
> both PGXACT entries have the same xid.
>
> You can reproduce that by putting a sleep or breakpoint in
> PrepareTransaction(), just before the
> "ProcArrayClearTransaction(MyProc);" call. If you call
> txid_current_snapshot() from another session at that point, it will
> output two duplicate xids. (you will have to also commit one more
> unrelated transaction to bump up xmax).

Thanks, that explains it.

>
>> At this point I would find it extremely helpful to "sanitize" the
>> external representation in txid_snapshot_out() while emitting some
>> NOTICE level logging when this actually happens. I am aware that this
>> does amount to a functional change for a back release, but considering
>> that the _out() generated external representation of an existing binary
>> datum won't pass the type's _in() function, I argue that such change is
>> warranted. Especially since this problem could possibly corrupt a dump.
>
> Hmm. Do we snapshots to be stored in tables, and included in a dump? I
> don't think we can guarantee that will work, at least not across
> versions, as the way we handle snapshot internally can change.

At least Londiste and Slony do store snapshots as well as xids in tables
and assuming that the txid epoch is properly bumped, that information is
useful and valid after a restore.

>
> But yeah, we probably should do something about that. The most
> straightforward fix would be to scan the array in
> txid_current_snapshot() and remove any duplicates.

The code in txid_snapshot_in() checks that the xip list is ascending.
txid_snapshot_out() does not sort the list, so it must already be sorted
when the snapshot itself is created. That scan would be fairly simple.

Jan

--
Jan Wieck
Senior Software Engineer
http://slony.info