Re: [HACKERS] logical decoding of two-phase transactions

Hi Petr, Andres and Tomas

>>> Thanks. I think the README is a good start, but I think we also need to
>>> improve the comments, which is usually more detailed than the README.
>>> For example, it's not quite acceptable that LogicalLockTransaction and
>>> LogicalUnlockTransaction have about no comments, especially when it's
>>> meant to be public API for decoding plugins.
>>

Tomas, thanks for providing your review comments based patch. I will include the
documentation that you have provided in that patch for the APIs. Will
also look at
your decodeGroupLocking related comments and submit a fresh patch soon.

>> FWIW, for me that's ground to not accept the feature. Burdening output
>> plugins with this will make their development painful (because they'll
>> have to adapt regularly) and correctness doubful (there's nothing
>> checking for the lock being skipped). Another way needs to be found.
>>
>
> I have to agree with Andres here.
>

Ok. Let's have another go at alleviating this issue then.

> I as wondering how to hide this. Best idea I had so far would be to put
> it in heap_beginscan (and index_beginscan given that catalog scans use
> is as well) behind some condition. That would also improve performance
> because locking would not need to happen for syscache hits. The problem
> is however how to inform the heap_beginscan about the fact that we are
> in 2PC decoding. We definitely don't want to change all the scan apis
> for this. I wonder if we could add some kind of property to Snapshot
> which would indicate this fact - logical decoding is using it's own
> snapshots it could inject the information about being inside the 2PC
> decoding.
>

The idea of adding that info in the Snapshot itself is interesting. We
could introduce a logicalxid field in SnapshotData to point to the XID
that the decoding backend is interested in. This could be added only
for the 2PC case. Support in the future for in-progress transactions
could use this field as well. If it's a valid XID, we could call
LogicalLockTransaction/LogicalUnlockTransaction on that XID from
heap_beginscan/head_endscan respectively. I can also look at what
other *_beginscan APIs would need this as well.

Regards,
Nikhils