Re: Logical Decoding and HeapTupleSatisfiesVacuum assumptions

On 22/01/18 20:21, Robert Haas wrote:
> On Mon, Jan 22, 2018 at 10:40 AM, Petr Jelinek
> <petr(dot)jelinek(at)2ndquadrant(dot)com> wrote:
>> I think this is the only real problem from your list for logical
>> decoding catalog snapshots. But it's indeed quite a bad one. Is there
>> something preventing us to remove the assumption that the CTID of T1 is
>> garbage nobody cares about? I guess turning off HOT for catalogs is not
>> an option :)
>
> It doesn't seem like a great idea. For a lot of workloads it wouldn't
> hurt, but for some it might cause a lot of catalog bloat. Also, Tom
> would probably hate it with a fiery passion, since as I understand it
> he argued passionately for HOT to work for both catalog and
> non-catalog tables back when it was first implemented.
>

I wasn't really serious about that proposal.

>
>> General problem is that we have couple of assumptions
>> (HeapTupleSatisfiesVacuum being one, what you wrote is another) about
>> tuples from aborted transactions not being read by anybody. But if we
>> want to add decoding of 2PC or transaction streaming that's no longer
>> true so I think we should try to remove that assumption (even if we do
>> it only for catalogs since that what we care about).
>
> I'm extremely skeptical about this idea. I think that assumption is
> fairly likely to be embedded in subtle ways in several more places
> that we haven't thought about yet. Unless we're very, very careful
> about doing something like this, we could end up with a system that
> mostly seems to work but is actually unreliable in ways that can't be
> fixed without non-back-patchable changes.

I am worried about it too, I don't feel like I can judge if we just need
to be very careful or if it's too big a risk so I'll defer to you here.
>> The other option would be to make sure 2PC decoding/tx streaming does
>> not read aborted transaction but that would mean locking the transaction
>> every time we give control to output plugin. Given that output plugin
>> may do network write, this would really mean locking the transaction for
>> and unbounded period of time. That does not strike me as something we
>> want to do, decoding should not interact with frontend transaction
>> management, definitely not this badly.
>
> Yeah, I don't think it's acceptable to postpone abort indefinitely.
>

Hmm, maybe less bad and potentially acceptable version of this would be
to make TransactionIdIsInProgress() treat transaction as running if it's
being decoded, that might solve most of the issues. There is still
potential interlocking issue with UPDATEs but they have to change same
tuples as the aborted tx did.

What I mean concretely is that decoding would check if tx has aborted,
if not then it would put somewhere in shmem info about it working on the
tx and once plugin is done with current call it would reset the shmem
info to invalid tx. Then the TransactionIdIsInProgress() would check
this shmem info before checking about abort. If the first check would
show that transaction is aborted then decoding would not bother decoding
it further.

--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services